microsoft / microsoft/mssql-rs

mssql-odbc: SQLSetDescRec/SQLSetDescFieldW can rewrite ARD/APD records mid-fetch with no STMT_STATE_FETCH_IN_PROGRESS guard

Open
#472 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
53
Forks
14
Avg merge
1d 15h
Merged PRs (30d)
137

Description

## Context

Follow-up from [PR #436](https://github.com/microsoft/mssql-rs/pull/436) ([AB#47437](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47437) — descriptor records as the binding/metadata source of truth), flagged by code review.

## Problem

Now that the ARD/APD records *are* the binding storage ([AB#47437](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47437)), `SQLSetDescRec` and `SQLSetDescFieldW` can rewrite `SQL_DESC_DATA_PTR` / `SQL_DESC_OCTET_LENGTH` / `SQL_DESC_CONCISE_TYPE` on a descriptor a fetch is actively using, with no `STMT_STATE_FETCH_IN_PROGRESS` guard. Every other entry point that touches the same records refuses in that window:

- `SQLBindCol` (`bind_col.rs:109`)
- `SQLFreeStmt(SQL_UNBIND)` (`bind_col.rs:315`)
- `SQLSetStmtAttr` (`set_stmt_attr.rs:164`)

`fetch_scroll_safe` snapshots `ColumnBinding::all_from_ard_state` before its fill loop, so the loop itself can't read a half-written record — but that snapshot is precisely why the guard exists on `SQLBindCol` in the first place: the point is that the application must not be able to hand the driver a *new* binding (and free the old buffer) while the fill loop is still writing through the snapshotted pointers. The descriptor-field API is now a second door into the same records, and before PR #436 it wasn't.

## Why this wasn't fixed directly in #436

A `DescHandle` has no back-pointer to the statement(s) it is associated with, so adding the guard to `SQLSetDescRec`/`SQLSetDescFieldW` needs a DBC → STMT walk, the same shape `free_desc` already uses to reset associations on free. Doing that walk for one door (the descriptor-field API) and not the others inconsistently, or rushing a partial version of it, risks being worse than the current, at-least-consistent gap. It needs the same careful DBC→STMT-ordered treatment as the rest of this crate's locking rules, not a quick patch alongside an already large PR.

## Suggested approach

Give `SQLSetDescRec`/`SQLSetDescFieldW` (`set_desc_rec.rs`, `set_desc_field.rs`) the same DBC → STMT walk `free_desc` uses (`.github/instructions/mssql-odbc.instructions.md`'s "Locking rules" section) to find every statement currently associated with the descriptor being modified (an explicit ARD/APD can be shared across statements via `SQL_ATTR_APP_ROW_DESC`/`SQL_ATTR_APP_PARAM_DESC` reassociation), and refuse with `ERR_FUNCTION_SEQUENCE` if any of them has `STMT_STATE_FETCH_IN_PROGRESS` set — matching `SQLBindCol`'s existing check.

## References

- PR: https://github.com/microsoft/mssql-rs/pull/436
- Review thread: https://github.com/microsoft/mssql-rs/pull/436#discussion_r3913342575
- Related work item: [AB#47437](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47437)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the locking rules in .github/instructions/mssql-odbc.instructions.md, then compare free_desc with SQLBindCol in bind_col.rs. Update set_desc_rec.rs and set_desc_field.rs to walk associated statements in DBC→STMT order and reject changes when any has STMT_STATE_FETCH_IN_PROGRESS, returning ERR_FUNCTION_SEQUENCE as SQLBindCol does.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.