Skip to content

Reconsider byte-for-byte msodbcsql parity for harmful connection-string parsing quirks (mssql-odbc) #114

Description

@saurabh500

Problem statement

The rewritten mssql-odbc connection-string parser (mssql-odbc/src/connection/connection_string_parser.rs, added in #107) deliberately reproduces msodbcsql's ParseAttrStr byte-for-byte, including its quirks. In review of #107, @David-Engel raised that several of these msodbcsql behaviors are "really bad" and lead to negative user experiences — they silently drop attributes or fail confusingly instead of erroring clearly.

The clearest example is the key scan reading through ;:

Input Old tokenizer msodbcsql ParseAttrStr (current mssql-odbc)
Server=h;bogus;UID=u bogus skipped, UID=u set key becomes bogus;UID (scan reads through ;); unknown → ignored; UID never set

Here a stray token silently swallows the following UID=u, so the user connects as an empty user and gets a login failure with no obvious cause. Per the ODBC connection-string spec (MS-ODBCSTR), keywords must not contain a space, {, or ; — so msodbcsql's "read through ;" behavior diverges from the spec. This was agreed on the #107 review thread to be handled as a follow-up.

Proposed solution

Take the mssql-odbc greenfield opportunity to deliberately diverge from msodbcsql for the genuinely harmful quirks, favoring spec-compliant, fail-loud behavior. For the buried-token case, either:

  • follow the old tokenizer behavior (treat ; as a hard key/value separator so Server=h;bogus;UID=u still sets UID=u), or
  • produce a clear error such as "Invalid keyword/value combination" rather than silently dropping the attribute.

As part of this, review the other intentionally-mirrored quirks documented in mssql-odbc/docs/connection_string_parser.md and decide, per behavior, whether to keep msodbcsql parity or fix it:

  • key scan reading through ; (buried token swallows the next attribute) — the primary case above
  • trailing separator run of 2+ posting 01S00 (see the parity work in Rewrite ODBC connection string parser to mirror msodbcsql ParseAttrStr #107)
  • structural malformations that stop parsing mid-string (no = in remainder, junk after a closing }, unterminated {)
  • keys not trimmed, so Server =h (space before =) silently fails to match

The outcome should be a documented, deliberate policy: for each quirk, "mirror msodbcsql" vs. "diverge for correctness/UX", with tests and docs updated to match.

Affected crate

mssql-odbc (not listed in the dropdown; closest: Not sure / Multiple)

Alternatives considered

  • Keep strict byte-for-byte msodbcsql parity for everything (status quo from Rewrite ODBC connection string parser to mirror msodbcsql ParseAttrStr #107). Rejected in review because it perpetuates spec-divergent, user-hostile behaviors in a brand-new driver where we are not bound by back-compat.
  • Fix everything silently without erroring. Weaker than failing loud, since silently "correcting" input can also surprise users; an explicit error for clearly-malformed input is preferable.

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions