FIX: Validate driver-provided fetch sizes by gargsaumya · Pull Request #803 · microsoft/mssql-python

gargsaumya · 2026-09-21T06:53:15Z

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.

…-validation

…nto saumya/native-fetch-validation

…nto saumya/native-fetch-validation # Conflicts: # mssql_python/pybind/ddbc_bindings.cpp

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.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…nto copilot/pr803-rebased

Copilot

🟡 Changes recommended

Direct LOB streaming still rejects valid SQL_NO_DATA completion after streamed progress.

1 open finding

Handle SQL_NO_DATA after chunked LOB reads · New

2 resolved since last review

🧠 Review effort: Balanced

Treat SQL_NO_DATA as normal completion only after a direct LOB fetch has returned payload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…crosoft/mssql-python into copilot/pr803-rebased

Copilot

🔵 Needs a closer look

The broad native ODBC changes require final cross-platform CI and human verification.

0 open findings

1 resolved since last review

Handle SQL_NO_DATA after chunked LOB reads

🧠 Review effort: Balanced

…crosoft/mssql-python into copilot/pr803-rebased

Copilot

🟡 Changes recommended

Reused or zero-initialized indicators can still accept omitted driver outputs and expose stale or incorrect results.

2 open findings

Initialize LOB length to an invalid sentinel · New
Reset indicator slots before each fetch · New

🧠 Review effort: Balanced

Seed direct and bound fetch indicators with invalid sentinels so omitted driver writes cannot expose empty or stale data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…crosoft/mssql-python into copilot/pr803-rebased

Copilot

🟡 Changes recommended

Several direct-fetch indicators remain initialized unsafely, allowing malformed successful responses to trigger undefined behavior or evade validation.

1 open finding

Initialize dataLen before SQLGetData · New

2 resolved since last review

🧠 Review effort: Balanced

Seed every direct SQLGetData output with the invalid indicator sentinel before driver calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…crosoft/mssql-python into copilot/pr803-rebased

Copilot

🟡 Changes recommended

Some malformed diagnostic and indicator paths still fail open or silently return None.

3 open findings

Fail closed when diagnostic retrieval fails · New
Reject missing sql_variant fetch indicator · New
Reject SQL_NO_TOTAL for fixed-width columns · New

1 resolved since last review

Initialize dataLen before SQLGetData

🧠 Review effort: Balanced

Reject unreadable truncation diagnostics, missing variant probe indicators, and SQL_NO_TOTAL for fixed-width columns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The direct decimal path still accepts a missing driver indicator and silently returns None.

0 open findings

3 resolved since last review

🧠 Review effort: Balanced

gargsaumya added 3 commits September 21, 2026 12:17

FIX: Validate driver-provided fetch sizes

6be230a

Merge branch 'saumya/native-size-validation' into saumya/native-fetch…

c58b9c2

…-validation

FIX: Unify checked size helpers

d085633

github-actions Bot added the pr-size: medium Moderate update size label Sep 21, 2026

gargsaumya added 5 commits September 21, 2026 12:58

Merge remote-tracking branch 'origin/saumya/native-size-validation' i…

8f69db5

…nto saumya/native-fetch-validation

Merge remote-tracking branch 'origin/saumya/native-size-validation' i…

f178c60

…nto saumya/native-fetch-validation # Conflicts: # mssql_python/pybind/ddbc_bindings.cpp

FIX: Validate driver-provided fetch sizes

b2c89b8

FIX: Preserve stacked fetch validation

d40dc89

Merge remote-tracking branch 'origin/saumya/native-size-validation' i…

522aebd

…nto saumya/native-fetch-validation # Conflicts: # mssql_python/pybind/ddbc_bindings.cpp

Sumit Sarabhai (sumitmsft) reviewed Sep 23, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Gaurav Sharma (bewithgaurav) requested changes Sep 24, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

gargsaumya and others added 2 commits October 7, 2026 11:41

FIX: Validate driver-provided fetch sizes

d9ebebb

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge prior native fetch validation history

a4de9e3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added pr-size: large Substantial code update and removed pr-size: medium Moderate update size labels Oct 7, 2026

Merge updated native size validation base

95d53ef

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added pr-size: medium Moderate update size and removed pr-size: large Substantial code update labels Oct 7, 2026

gargsaumya and others added 2 commits October 7, 2026 11:46

Merge main-updated native size validation base

319bcfe

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge peak-safe native size validation base

12d688f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

gargsaumya requested a balanced review from Copilot October 7, 2026 06:25

Copilot started reviewing on behalf of gargsaumya October 7, 2026 06:26 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Comment thread mssql_python/pybind/ddbc_bindings.cpp

Merge bounded LOB native size base

6e1efed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

gargsaumya requested a balanced review from Copilot October 7, 2026 06:31

Copilot started reviewing on behalf of gargsaumya October 7, 2026 06:31 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

gargsaumya and others added 2 commits October 7, 2026 12:08

FIX: Harden malformed fetch handling

b441a0d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'origin/saumya/native-size-validation' i…

6936472

…nto copilot/pr803-rebased

gargsaumya requested a balanced review from Copilot October 7, 2026 06:41

Merge remote-tracking branch 'origin/saumya/native-size-validation' i…

969563e

…nto copilot/pr803-rebased

Subrata (subrata-ms) reviewed Oct 8, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Subrata (subrata-ms) reviewed Oct 8, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

gargsaumya requested a balanced review from Copilot October 12, 2026 05:02

Copilot started reviewing on behalf of gargsaumya October 12, 2026 05:03 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp

gargsaumya and others added 2 commits October 12, 2026 12:25

FIX: accept SQL_NO_DATA after LOB progress

9e54e19

Treat SQL_NO_DATA as normal completion only after a direct LOB fetch has returned payload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge branch 'saumya/native-size-validation' of http://github.com/mi…

f8c8ce5

…crosoft/mssql-python into copilot/pr803-rebased

gargsaumya requested a balanced review from Copilot October 12, 2026 06:58

Copilot started reviewing on behalf of gargsaumya October 12, 2026 06:59 View session

Merge branch 'saumya/native-size-validation' of http://github.com/mi…

db79491

…crosoft/mssql-python into copilot/pr803-rebased

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Merge branch 'saumya/native-size-validation' of http://github.com/mi…

4bceee4

…crosoft/mssql-python into copilot/pr803-rebased

gargsaumya requested a balanced review from Copilot October 12, 2026 07:10

Copilot started reviewing on behalf of gargsaumya October 12, 2026 07:11 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp

Comment thread mssql_python/pybind/ddbc_bindings.cpp

gargsaumya and others added 2 commits October 12, 2026 12:48

FIX: invalidate driver-written indicators

153016a

Seed direct and bound fetch indicators with invalid sentinels so omitted driver writes cannot expose empty or stale data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge branch 'saumya/native-size-validation' of http://github.com/mi…

cef1fe0

…crosoft/mssql-python into copilot/pr803-rebased

gargsaumya requested a balanced review from Copilot October 12, 2026 07:21

Copilot started reviewing on behalf of gargsaumya October 12, 2026 07:21 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp

gargsaumya and others added 2 commits October 12, 2026 15:03

FIX: initialize direct fetch indicators

b4ceb6b

Seed every direct SQLGetData output with the invalid indicator sentinel before driver calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge branch 'saumya/native-size-validation' of http://github.com/mi…

200193a

…crosoft/mssql-python into copilot/pr803-rebased

gargsaumya requested a balanced review from Copilot October 12, 2026 09:35

Copilot started reviewing on behalf of gargsaumya October 12, 2026 09:36 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Comment thread mssql_python/pybind/ddbc_bindings.cpp

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

FIX: fail closed on malformed fetch metadata

58ae087

Reject unreadable truncation diagnostics, missing variant probe indicators, and SQL_NO_TOTAL for fixed-width columns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

gargsaumya requested a balanced review from Copilot October 12, 2026 09:45

Copilot started reviewing on behalf of gargsaumya October 12, 2026 09:46 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Conversation

gargsaumya commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Work Item / Issue Reference

Summary

Uh oh!

Uh oh!

Gaurav Sharma (bewithgaurav) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gargsaumya commented Sep 21, 2026 •

edited

Loading