FIX: Validate driver-provided fetch sizes

[gargsaumya]

Work Item / Issue Reference

ADO Work Item: AB#47843

---

Summary

• Validate ODBC row counts, indicators, and column-buffer layout before access.
• Check streamed fetch growth and Arrow source/destination bounds.
• Correct fixed-width buffer lengths and reject malformed wide-character data.

Stacked on #802 so this review contains only driver-provided fetch validation.

[Gaurav Sharma (bewithgaurav)]

the driver-provided size validation is well scoped and the normal fetch and arrow paths hold up. I found one suggestion worth incorporating before merge, so requesting changes.

[Copilot]

Copilot review overview

🟡 Changes recommended

Successful zero-row fetches can loop indefinitely, and malformed indicators or the global test hook can cause resource and concurrency problems.

Review effort: Balanced
Findings: 2 · 2

Open (4)

• Reject successful zero-row fetches to prevent infinite loops · New
• Avoid unsynchronized mutation of the global SQLGetData hook · New
• Validate data length before growing Arrow destination · New
• Check source bounds before allocating from data length · New

What changed in this PR

Adds defensive validation for driver-provided fetch metadata in the native ODBC layer.

Changes:

• Validates row counts, indicators, wide-character lengths, and Arrow bounds.
• Hardens streamed variable-length fetching and buffer growth.
• Adds native regression scenarios and subprocess coverage.

File	Description
mssql_python/​pybind/​ddbc_bindings.cpp	Implements fetch validation and test hooks.
mssql_python/​pybind/​ddbc_bindings.h	Rejects malformed wide-character lengths.
tests/​test_010_pybind_functions.py	Tests malformed driver responses and streaming progress.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The test hook unsafely replaces a process-wide ODBC function pointer while concurrent fetches may access it without the GIL.

Review effort: Balanced
Findings: 2 · 2

Open (4)

• Avoid unsynchronized mutation of the global SQLGetData hook
• Reject successful zero-row fetches to prevent infinite loops
• Check source bounds before allocating from data length
• Validate data length before growing Arrow destination

[Copilot]

Copilot review overview

🟡 Changes recommended

LOB streaming can still accept SQL_NO_TOTAL without a truncation diagnostic and return fabricated chunk data.

Review effort: Balanced
Findings: 2

Open (2)

• Reject SQL_NO_TOTAL without 01004 truncation diagnostic · New
• Prevent infinite retry when trimmed truncation response has no payload

[Copilot]

Copilot review overview

🔵 Needs a closer look

Native ODBC safety changes across multiple fetch paths require final human and cross-platform runtime validation.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Reject SQL_NO_TOTAL without 01004 truncation diagnostic
• Prevent infinite retry when trimmed truncation response has no payload

[Copilot]

Copilot review overview

🔵 Needs a closer look

Native ODBC buffer handling across DB-API and Arrow paths requires final human and cross-platform runtime validation.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

The native ODBC and Arrow safety changes span several fetch paths and require cross-platform driver-backed validation.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

Native ODBC buffer-safety changes require cross-platform runtime validation with the compiled extension and SQL Server.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🟡 Changes recommended

Fixed-width indicators remain unvalidated in row and Arrow fetch paths.

Review effort: Balanced
Findings: 1

Open (1)

• Validate fixed-width indicators before reading reused buffers · New

[Copilot]

Copilot review overview

🟡 Changes recommended

Direct fixed-width fetches remain unvalidated, and direct LOB streaming mishandles valid SQL_NO_DATA completion.

Review effort: Balanced
Findings: 1

Open (1)

• Validate lengths in direct fixed-width SQLGetData conversions · New

Resolved since last review (1)

• Validate fixed-width indicators before reading reused buffers

Previously missed (1)

In code that hasn't changed since last review

Handle SQL_NO_DATA after SQL_NO_TOTAL LOB chunks

mssql_python/​pybind/​ddbc_bindings.cpp:3743

This completion logic still omits SQL_NO_DATA. After a truncating SQL_NO_TOTAL chunk, ODBC may complete the sequence with SQL_NO_DATA; unlike GetDataVar at lines 5676-5686, this direct LOB path treats that status as an error before reaching this break and raises for an otherwise completed value. Handle SQL_NO_DATA before reading the now-undefined indicator, accepting it only after payload progress, and add the corresponding injected LOB sequence.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Fixed-width validation adds redundant type dispatch within both row and Arrow per-cell hot loops.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Validate lengths in direct fixed-width SQLGetData conversions

Previously missed (1)

In code that hasn't changed since last review

Avoid per-cell type dispatch for invariant column widths

mssql_python/​pybind/​ddbc_bindings.cpp:5180

This restores a type switch for every fetched cell, even though dataType is invariant per column and the dispatch table above explicitly precomputes shape-dependent work to avoid that hot-loop cost. Precompute the expected fixed width in ColumnInfo while caching metadata, then make this per-cell validation a simple zero-or-equality check; that preserves the new safety check without adding repeated type dispatch to rows × columns processing.

This issue also appears on line 6644 of the same file.