Skip to content

GH-50852: [C++][FlightRPC][ODBC] Decode wide connection/descriptor string attributes with the wide decoder - #50853

Open
vikrantpuppala wants to merge 1 commit into
apache:mainfrom
vikrantpuppala:GH-wide-attr-decode-fix
Open

GH-50852: [C++][FlightRPC][ODBC] Decode wide connection/descriptor string attributes with the wide decoder#50853
vikrantpuppala wants to merge 1 commit into
apache:mainfrom
vikrantpuppala:GH-wide-attr-decode-fix

Conversation

@vikrantpuppala

@vikrantpuppala vikrantpuppala commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Two string attributes in the ODBC driver are decoded with the byte-wise
decoder even when the value arrives through a wide (Unicode / *W) entry point,
so the wide buffer is misread and corrupted.

ODBCConnection::SetConnectAttr(SQL_ATTR_CURRENT_CATALOG) had its two decode
branches swapped:

if (is_unicode) {
  SetAttributeUTF8(value, string_length, catalog);      // byte-wise
} else {
  SetAttributeSQLWCHAR(value, string_length, catalog);  // wide
}

is_unicode selects the buffer width: a unicode (*W) call passes a wide
SQLWCHAR buffer that must be decoded with SetAttributeSQLWCHAR; a non-unicode
call passes a byte string decoded with SetAttributeUTF8. With the branches
swapped, a wide catalog name (e.g. UTF-16 "my_catalog") is reinterpreted
byte-wise and stored with the wide encoding's embedded NULs
("m\0y\0_\0c\0a\0t\0a\0l\0o\0g\0"), so catalog scoping operates on a corrupt
name. This is the inverse of the mapping the getter already uses:
GetStringAttribute maps is_unicode -> GetAttributeSQLWCHAR.

ODBCDescriptor::SetField(SQL_DESC_NAME) is the same class of bug: it
unconditionally used the byte-wise SetAttributeUTF8, even though the matching
getter (GetField / SQL_DESC_NAME) reads the field back with
GetAttributeSQLWCHAR.

How this happened (history):

What changes are included in this PR?

  • odbc_connection.cc: swap the SQL_ATTR_CURRENT_CATALOG decode branches so a
    unicode call uses SetAttributeSQLWCHAR and a non-unicode call uses
    SetAttributeUTF8, matching GetStringAttribute.
  • odbc_descriptor.cc: decode SQL_DESC_NAME with SetAttributeSQLWCHAR to
    match its getter.
  • connection_attr_test.cc: add TestSQLSetGetConnectAttrCurrentCatalogWide, a
    TYPED_TEST (mock + remote fixtures) that sets a multi-character catalog
    through the wide entry point and asserts the full name round-trips back.

Are these changes tested?

Yes. The new test is a set-then-get round-trip through the real driver stack
(SQLSetConnectAttr / SQLGetConnectAttr), placed next to the other connection
attribute tests, and it distinguishes the fixed and unfixed driver:

check result
new test, with fix (mock fixture) PASS — out_catalog == "my_catalog"
new test, without fix (mock fixture) FAIL — out_catalog == "m\0y\0_\0c\0a\0t\0a\0l\0o\0g\0"
ConnectionAttributeTest/0.* (full mock suite) 25/25 pass
clang-format clean on all three files

The mock fixture (FlightSQLODBCMockTestBase) runs against an in-process SQLite
Flight SQL server; the remote variant runs when ARROW_FLIGHT_SQL_ODBC_CONN is
set. Verified locally against the mock fixture.

Are there any user-facing changes?

Yes. A catalog name (and descriptor SQL_DESC_NAME) set through the wide entry
point is now decoded correctly instead of being corrupted. Applications that set
a multi-character catalog through SQLSetConnectAttrW now scope to the intended
catalog.

AI usage

Per the AI-generated code guidance:
the diagnosis, the fix, and the test were produced with the assistance of an AI
coding agent and reviewed and verified by me. Correctness was checked by
(1) confirming the getter path (GetStringAttribute) maps is_unicode to the
wide decoder, establishing the intended mapping the setters violated, and
(2) running the new round-trip test against both the fixed and unfixed driver to
confirm it distinguishes them (clean name vs. embedded-NUL corruption).


🤖 Drafted by Claude Code (an AI agent) and reviewed & verified by vikrantpuppala.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50852 has been automatically assigned in GitHub to PR creator.

…tor string attributes with the wide decoder

SetConnectAttr(SQL_ATTR_CURRENT_CATALOG) had its is_unicode decode branches
swapped, and SetField(SQL_DESC_NAME) unconditionally used the byte-wise
SetAttributeUTF8. In both cases a wide SQLWCHAR buffer was misread, corrupting
the stored value. Decode wide buffers with SetAttributeSQLWCHAR, matching the
mapping the getters already use (GetStringAttribute / GetAttributeSQLWCHAR).

Add a round-trip TYPED_TEST for SQL_ATTR_CURRENT_CATALOG through the wide entry
point in connection_attr_test.cc.

Co-authored-by: Isaac

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Corrects wide-string decoding for ODBC catalog and descriptor attributes.

Changes:

  • Uses the wide decoder for Unicode catalog and descriptor names.
  • Preserves UTF-8 decoding for non-Unicode catalog values.
  • Adds catalog round-trip coverage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
connection_attr_test.cc Tests wide catalog round-tripping.
odbc_descriptor.cc Corrects descriptor-name decoding.
odbc_connection.cc Corrects catalog decoder selection.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

break;
case SQL_DESC_NAME:
SetAttributeUTF8(value, buffer_length, record.name);
SetAttributeSQLWCHAR(value, buffer_length, record.name);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants