Add mssql-python connection-string parity attributes to ODBC parser by Vahid-b · Pull Request #147 · microsoft/mssql-rs

Vahid-b · 2026-07-23T03:04:57Z

Description

Finishes connection-string compatibility parity with mssql-python (feature AB#46372) by extending the SQLDriverConnect parser to recognize and act on the full canonical ODBC Driver 18 keyword set that mssql-python normalizes to before handing the string to the driver.

Newly acted-on keywords, each wired into the mssql-tds ClientContext / EncryptionOptions:

Keyword(s)	Landing spot	Value handling
`ServerSPN`	`server_spn`	verbatim
`ApplicationIntent`	`application_intent`	`ReadOnly` / `ReadWrite` (validated)
`MultiSubnetFailover`	`multi_subnet_failover`	`Yes` / `No` (validated)
`ConnectRetryCount` / `ConnectRetryInterval`	matching fields	integer
`KeepAlive` / `KeepAliveInterval`	`keep_alive_in_ms` / `keep_alive_interval_in_ms`	integer seconds → ms (×1000)
`IpAddressPreference`	`ipaddress_preference`	`IPv4First` / `IPv6First` / `UsePlatformDefault` (validated)
`PacketSize`	`packet_size`	integer bytes (→ u16, saturating)
`HostnameInCertificate`	`EncryptionOptions::host_name_in_cert`	verbatim
`ServerCertificate`	`EncryptionOptions::server_certificate`	verbatim path → `PathBuf`

Also:

Map the msodbcsql-native Addr / Address synonyms to Server (they were previously in the ignored set and silently dropped).
Enumerated keys hard-reject invalid values (E_FAIL → HY024), matching how Encrypt/Yes-No already behave. Integer keys parse leniently — rejecting only non-numeric/negative input and leaving documented range clamping to mssql-tds so the parser can't diverge from the native driver.
Collapse the KEY_* consts and the classify_key match into a single MAPPED_KEYS descriptor table (the code's own TODO sanctioned this once the mapped set grew).

Value validation ownership note: mssql-python value-validates nothing itself (it only maps Authentication for client-side token acquisition and passes everything else through), so mssql-odbc owns all connection-string value validation — which this PR implements.

Related Issues

Closes AB#46418
Feature: AB#46372
Work item: https://dev.azure.com/SqlClientDrivers/mssql-rs/_workitems/edit/46418

Checklist

cargo bfmt passes
cargo bclippy passes
cargo btest passes — mssql-odbc suite is green (337/337 via nextest). The remaining workspace failures are mssql-tds live-DB integration tests that require a .env/SQL Server (untouched crate, environmental).
New/changed functionality has tests — 14 new parser unit tests + a gated end-to-end NewConnectionAttributesParity test
Public API changes are documented — docs/connection_string_parser.md updated (mapped-attributes table + value validation)

Recognize and act on the canonical ODBC Driver 18 keywords that mssql-python normalizes to before handing the string to the driver: ServerSPN, ApplicationIntent, MultiSubnetFailover, ConnectRetryCount, ConnectRetryInterval, KeepAlive, KeepAliveInterval, IpAddressPreference, PacketSize, HostnameInCertificate, and ServerCertificate. Map the msodbcsql-native Addr/Address synonyms to Server (previously silently dropped). Wire the new fields into the mssql-tds ClientContext and EncryptionOptions, converting KeepAlive seconds to milliseconds and ServerCertificate to a path. Enumerated keys (ApplicationIntent, MultiSubnetFailover, IpAddressPreference) validate their values and hard-reject invalid input (E_FAIL); integer keys parse leniently, rejecting only non-numeric or negative input and leaving range clamping to mssql-tds. Collapse the KEY_* consts and the classify_key match into a single MAPPED_KEYS descriptor table. Closes AB#46418. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-23T03:58:28Z

📊 Code Coverage Report

🔥 Diff Coverage 98%	🎯 Overall Coverage 90.3%	📦 Project: `mssql-tds + mssql-odbc + mssql-py-core` ℹ️ Note: diff coverage is reported, not enforced.

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

mssql-odbc/src/api/driver_connect.rs (96.4%): Missing lines 287,365,389-390,392
mssql-odbc/src/connection/connection_string_parser.rs (100%)

Summary

Total: 356 lines
Missing: 5 lines
Coverage: 98%

mssql-odbc/src/api/driver_connect.rs

  283         host_name_in_cert: None,
  284         server_certificate: None,
  285     };
  286 
! 287     apply_connection_params(&mut context, &params);
  288 
  289     // Connect via mssql-tds (lock is NOT held - the 'Connecting' state prevents races)
  290     let provider = TdsConnectionProvider::new();
  291     let client = dbc

  361     if let Some(intent) = &params.application_intent {
  362         context.application_intent = if intent.eq_ignore_ascii_case("readonly") {
  363             ApplicationIntent::ReadOnly
  364         } else {
! 365             ApplicationIntent::ReadWrite
  366         };
  367     }
  368     if let Some(multi_subnet_failover) = params.multi_subnet_failover {
  369         context.multi_subnet_failover = multi_subnet_failover;

  385     }
  386     if let Some(pref) = &params.ip_address_preference {
  387         context.ipaddress_preference = if pref.eq_ignore_ascii_case("ipv6first") {
  388             IPAddressPreference::IPv6First
! 389         } else if pref.eq_ignore_ascii_case("useplatformdefault") {
! 390             IPAddressPreference::UsePlatformDefault
  391         } else {
! 392             IPAddressPreference::IPv4First
  393         };
  394     }
  395     if let Some(size) = params.packet_size {
  396         context.packet_size =

🔗 Quick Links

View Azure DevOps Build · Coverage Report

Vahid-b · 2026-07-23T15:36:10Z

/azp run

azure-pipelines · 2026-07-23T15:36:28Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Extends ODBC connection-string parsing and TDS context mapping for mssql-python compatibility.

Changes:

Adds and validates canonical connection attributes and synonyms.
Maps parsed values into ClientContext and EncryptionOptions.
Adds parser documentation and tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`mssql-odbc/src/connection/connection_string_parser.rs`	Parses, validates, and formats new attributes.
`mssql-odbc/src/api/driver_connect.rs`	Applies attributes to TDS connection settings.
`mssql-odbc/docs/connection_string_parser.md`	Documents supported attributes and validation.
`mssql-odbc/tests/e2e/tests/driver_connect_test.cpp`	Adds live connection coverage.

Comments suppressed due to low confidence (1)

mssql-odbc/src/api/driver_connect.rs:327

mssql-tds does not clamp this value: ClientContext::validate rejects packet sizes outside 512–32768 before connecting. Values such as PacketSize=40000 therefore fail with 08001, while the PR promises lenient range clamping. Clamp to the actual TDS range before conversion.

        context.packet_size = u16::try_from(size).unwrap_or(u16::MAX);

Vahid-b · 2026-07-23T17:40:13Z

+    if let Some(count) = params.connect_retry_count {
+        context.connect_retry_count = count;


Good catch. Fixed in 68f25f7: the param->ClientContext mapping now lives in apply_connection_params, which clamps ConnectRetryCount to 0-255 (and ConnectRetryInterval to 1-60) at the boundary, so the downstream connect_retry_count + 1 in mssql-tds can no longer overflow. Clamping rather than rejecting mirrors msodbcsql. Added a unit test that feeds u32::MAX and asserts the result stays addable.

Vahid-b · 2026-07-23T17:40:15Z


+/// Parse a non-negative integer attribute value. Rejects non-numeric or negative
+/// input with `InvalidAttrValue` (msodbcsql `E_FAIL`). Documented per-key ranges
+/// (e.g. PacketSize 512–32767) are intentionally not enforced here; range handling


Fixed. The doc comment now reads 512-32768 (matching DefaultClientContextValidator, MAX = 32768) and no longer claims range handling is deferred to mssql-tds.

Vahid-b · 2026-07-23T17:40:12Z

+  `KeepAliveInterval`, `PacketSize`): a non-negative integer. Documented ranges are
+  **not** enforced at parse time — out-of-range clamping is left to mssql-tds so the
+  parser cannot diverge from the native driver. A non-numeric or negative value is a


Updated. The design doc now describes the real behavior: the parser accepts any u32 and the driver clamps out-of-range values when mapping onto the ClientContext (PacketSize 512-32768, ConnectRetryCount 0-255, ConnectRetryInterval 1-60), rather than delegating range handling to mssql-tds.

Vahid-b · 2026-07-23T17:40:18Z

+        if let Some(server_certificate) = &self.server_certificate {
+            parts.push(format!("ServerCertificate={server_certificate}"));


Addressed in 68f25f7. This formatter only feeds the redacted diagnostic log line (never re-parsed), but the ambiguity was real. Added a quote_odbc_value helper that brace-quotes values containing ;{}= and doubles any inner }, applied to the free-form fields including ServerCertificate, with tests.

Vahid-b · 2026-07-23T17:40:16Z

+        host_name_in_cert: params.host_name_in_certificate.clone(),
+        server_certificate: params.server_certificate.as_deref().map(PathBuf::from),


Done. Factored the mapping into apply_connection_params(&mut ClientContext, &ConnectionParams) and added direct unit tests asserting the HostnameInCertificate/ServerCertificate -> EncryptionOptions mappings, plus the enum and clamping paths, so they are no longer only reachable via a live-server e2e test.

Address Copilot review on PR #147: - Extract apply_connection_params() so the param->ClientContext mapping is unit-testable, and clamp ConnectRetryCount (0-255), ConnectRetryInterval (1-60), and PacketSize (512-32768) at the TDS boundary. Clamping mirrors msodbcsql and prevents the downstream connect_retry_count + 1 overflow in mssql-tds. - Brace-quote free-form values containing ; { } = in the redacted diagnostic connection-string log so delimiters stay unambiguous. - Fix parse_uint doc (32767 -> 32768) and the design doc's stale 'clamping left to mssql-tds' claim to describe the actual clamping behavior. - Add unit tests for the mapping, clamping, and value quoting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

shiwanigupta0809 · 2026-07-26T09:58:25Z

+            ));
+        }
+        if let Some(connect_retry_count) = self.connect_retry_count {
+            parts.push(format!("ConnectRetryCount={connect_retry_count}"));


in msodbcsql ConnectRetryCount and ConnectRetryInterval are range-validated during parse and rejected when out of range, but here we accept and clamp later. Is this clamping intentional here?
https://sqlclientdrivers.visualstudio.com/msodbcsql/_git/msodbcsql?path=/Sql/Ntdbms/sqlncli/odbc/sqlcconn.cpp&version=GBmaster&_a=contents&line=4136&lineStyle=plain&lineEnd=4136&lineStartColumn=9&lineEndColumn=36
https://sqlclientdrivers.visualstudio.com/msodbcsql/_git/msodbcsql?path=/Sql/Ntdbms/sqlncli/odbc/sqlcconn.cpp&version=GBmaster&_a=contents&line=4009&lineStyle=plain&lineEnd=4009&lineStartColumn=1&lineEndColumn=30

shiwanigupta0809 · 2026-07-26T10:52:54Z

+            params.keep_alive_interval = Some(parse_uint(lower, value)?);
+        }
+        ConnAttrKey::IpAddressPreference => {
+            validate_attr(lower, value, IP_ADDRESS_PREFERENCE_VALUES)?;


msodbcsql falls back unknown values to IPv4First, while this parser currently rejects unknown values with HY024. Is this strict rejection intentional?
I didnt find ODBC spec defined behavior for unknown IpAddressPreference values, so this is a product-parity decision

Vahid-b requested a review from Copilot July 23, 2026 16:42

Copilot started reviewing on behalf of Vahid-b July 23, 2026 16:43 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Vahid-b marked this pull request as ready for review July 24, 2026 16:30

Vahid-b requested a review from a team as a code owner July 24, 2026 16:30

Vahid-b requested review from David-Engel and saurabh500 July 24, 2026 16:47

Vahid-b enabled auto-merge (squash) July 24, 2026 16:47

shiwanigupta0809 reviewed Jul 26, 2026

View reviewed changes

		if let Some(count) = params.connect_retry_count {
		context.connect_retry_count = count;

		if let Some(server_certificate) = &self.server_certificate {
		parts.push(format!("ServerCertificate={server_certificate}"));

		host_name_in_cert: params.host_name_in_certificate.clone(),
		server_certificate: params.server_certificate.as_deref().map(PathBuf::from),

Uh oh!

Conversation

Vahid-b commented Jul 23, 2026 • edited by azure-boards Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

🔥 Diff Coverage

98%

🎯 Overall Coverage

90.3%

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

Summary

mssql-odbc/src/api/driver_connect.rs

🔗 Quick Links

Uh oh!

Vahid-b commented Jul 23, 2026

Uh oh!

azure-pipelines Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Vahid-b Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Vahid-b Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Vahid-b Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Vahid-b Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Vahid-b Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

shiwanigupta0809 Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shiwanigupta0809 Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Vahid-b commented Jul 23, 2026 •

edited by azure-boards Bot

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading

shiwanigupta0809 Jul 26, 2026 •

edited

Loading