From 538a720c0cb0a533fcda3b859275d5e79f2a6b42 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Thu, 14 May 2026 22:22:48 -0300 Subject: [PATCH] Fix SQL_C_GUID parameter binding sending corrupted data on the wire When an ODBC application called SQLBindParameter with C type SQL_C_GUID and SQL type SQL_GUID, the driver accepted the call but did not convert the 16-byte UUID into Firebird's wire format. Symptoms reported in #295: * BINARY(16) / CHAR(16) CHARACTER SET OCTETS targets received the ASCII bytes of the canonical UUID string truncated to 16 chars. * Untyped VARCHAR parameters (e.g. inside CHAR_TO_UUID(?)) received a wide-char buffer interpreted as narrow text, so Firebird raised "Human readable UUID argument for CHAR_TO_UUID must have hex digit at position 2 instead of ''". Add two wire-only conversion functions and route SQL_C_GUID input parameters to them: * convGuidToBinary writes 16 raw bytes in canonical UUID byte order (Data1/2/3 swapped from x86 little-endian to big-endian, Data4 as is). Used for BINARY(16) / CHAR(16) OCTETS / FB4+ BINARY targets. * convGuidToVarString stages the 36-char canonical UUID in the DescRecord local buffer and redirects the wire's sqldata via setSqlData(), matching the idiom transferStringToAllowedType already uses. This sidesteps the SQLDA-allocated buffer being only 2 bytes wide for an untyped `?` (VARCHAR(0)) placeholder. The pre-existing convGuidToString / convGuidToStringW remain unchanged; they handle the (currently unreachable) app-side fetch path that will become live with the column-side SQL_GUID mapping work tracked under #287 T5-5. Expose getSqlSubtype() and getSqlLen() as read-only accessors on HeadSqlVar so the dispatch in getAdressFunction can distinguish sqlsubtype == 1 + sqllen == 16 (BINARY/CHAR(16) OCTETS, raw bytes) from text wires of any other charset (canonical UUID string). Add three GuidParamBindingTest acceptance tests covering the issue's exact reproducers: SQL_C_GUID into CHAR(16) OCTETS, into VARCHAR via CHAR_TO_UUID(?), and into BINARY(16) via UUID_TO_CHAR(?). Closes #295. --- IscDbc/Connection.h | 2 + IscDbc/IscHeadSqlVar.h | 2 + OdbcConvert.cpp | 100 +++++++++++++++++++++++ OdbcConvert.h | 2 + tests/test_guid_and_binary.cpp | 141 +++++++++++++++++++++++++++++++++ 5 files changed, 247 insertions(+) diff --git a/IscDbc/Connection.h b/IscDbc/Connection.h index 8c980fb2..8ec28734 100644 --- a/IscDbc/Connection.h +++ b/IscDbc/Connection.h @@ -524,6 +524,8 @@ class HeadSqlVar //virtual void setSqlSubType ( short subtype ) = 0; virtual void setSqlLen ( short len ) = 0; virtual short getSqlMultiple () = 0; + virtual short getSqlSubtype () = 0; + virtual short getSqlLen () = 0; virtual char * getSqlData() = 0; virtual short * getSqlInd() = 0; diff --git a/IscDbc/IscHeadSqlVar.h b/IscDbc/IscHeadSqlVar.h index 38fb53b8..debbe95f 100644 --- a/IscDbc/IscHeadSqlVar.h +++ b/IscDbc/IscHeadSqlVar.h @@ -100,6 +100,8 @@ class IscHeadSqlVar : public HeadSqlVar inline void setSqlData ( char* data ) { sqlvar->sqldata = data; } inline short getSqlMultiple () { return sqlMultiple; } + inline short getSqlSubtype () { return sqlvar->sqlsubtype; } + inline short getSqlLen () { return sqlvar->sqllen; } inline char * getSqlData() { return sqlvar->sqldata; } inline short * getSqlInd() { return sqlvar->sqlind; } diff --git a/OdbcConvert.cpp b/OdbcConvert.cpp index 1f5a8594..3ca9499c 100644 --- a/OdbcConvert.cpp +++ b/OdbcConvert.cpp @@ -1001,12 +1001,38 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t } break; case SQL_C_GUID: + // Wire-side (input parameter binding) — Firebird's IPD describes the slot + // as either BINARY(16) / CHAR(16) OCTETS, a text VARCHAR/CHAR, or (FB4+) + // genuine BINARY. Route to dedicated wire-only conversions; leave the + // pre-existing convGuidToString / convGuidToStringW untouched for the + // app-side fetch path below. + if ( to->isIndicatorSqlDa ) + { + // BINARY(16) / CHAR(16) CHARACTER SET OCTETS — 16 raw bytes + if ( to->headSqlVarPtr->getSqlSubtype() == 1 + && to->headSqlVarPtr->getSqlLen() == 16 ) + return &OdbcConvert::convGuidToBinary; + // FB4+ BINARY/VARBINARY described directly as SQL_C_BINARY + if ( to->conciseType == SQL_C_BINARY ) + return &OdbcConvert::convGuidToBinary; + // Text wire (VARCHAR/CHAR, any charset) — 36-char canonical UUID. + // setTypeText() converts SQL_VARYING to SQL_TEXT so convGuidToVarString + // can write the bytes without a length prefix. + to->headSqlVarPtr->setTypeText(); + return &OdbcConvert::convGuidToVarString; + } + // App-side output (fetching a column described as SQL_GUID into the + // application's bound buffer). Currently unreachable because the column + // side of SQL_GUID mapping is task T5-5 (open), but kept intact for when + // that lands. switch(to->conciseType) { case SQL_C_CHAR: return &OdbcConvert::convGuidToString; case SQL_C_WCHAR: return &OdbcConvert::convGuidToStringW; + case SQL_C_BINARY: + return &OdbcConvert::convGuidToBinary; default: return &OdbcConvert::notYetImplemented; } @@ -1688,6 +1714,80 @@ int OdbcConvert::convGuidToStringW(DescRecord * from, DescRecord * to) return SQL_SUCCESS; } +// Write SQLGUID as 16 bytes in canonical UUID byte order (Data1/2/3 big-endian, Data4 unchanged). +// Used when the target is BINARY(16) / CHAR(16) CHARACTER SET OCTETS or SQL_C_BINARY. +int OdbcConvert::convGuidToBinary(DescRecord * from, DescRecord * to) +{ + char* pointer = (char*)getAdressBindDataTo((char*)to->dataPtr); + SQLLEN * indicatorTo = getAdressBindIndTo((char*)to->indicatorPtr); + SQLLEN * indicatorFrom = getAdressBindIndFrom((char*)from->indicatorPtr); + + ODBCCONVERT_CHECKNULL( pointer ); + + SQLGUID *g = (SQLGUID*)getAdressBindDataFrom((char*)from->dataPtr); + + unsigned char buf[16]; + buf[0] = (unsigned char)((g->Data1 >> 24) & 0xFF); + buf[1] = (unsigned char)((g->Data1 >> 16) & 0xFF); + buf[2] = (unsigned char)((g->Data1 >> 8) & 0xFF); + buf[3] = (unsigned char)( g->Data1 & 0xFF); + buf[4] = (unsigned char)((g->Data2 >> 8) & 0xFF); + buf[5] = (unsigned char)( g->Data2 & 0xFF); + buf[6] = (unsigned char)((g->Data3 >> 8) & 0xFF); + buf[7] = (unsigned char)( g->Data3 & 0xFF); + memcpy(buf + 8, g->Data4, 8); + + int outlen = (int)to->length; + int len = outlen < 16 ? outlen : 16; + + if ( len > 0 ) + memcpy(pointer, buf, len); + + if ( to->isIndicatorSqlDa ) { + to->headSqlVarPtr->setSqlLen(len); + } else + if ( indicatorTo ) + setIndicatorPtr(indicatorTo, len, to); + + return SQL_SUCCESS; +} + +// Write SQLGUID as the 36-char canonical UUID string into a Firebird text +// parameter slot (VARCHAR/CHAR of any text charset). The wire-side SQLDA +// buffer for an untyped `?` placeholder is sized for VARCHAR(0) — too small +// for 36 bytes — so we stage the string in the DescRecord's local buffer +// and redirect Firebird to read from there via setSqlData(), mirroring the +// idiom transferStringToAllowedType already uses for app→wire string moves. +// The dispatch in getAdressFunction has already called setTypeText() to +// convert SQL_VARYING wires to SQL_TEXT (no length prefix), so we just +// write 36 raw bytes and set sqllen. +int OdbcConvert::convGuidToVarString(DescRecord * from, DescRecord * to) +{ + SQLLEN * indicatorFrom = getAdressBindIndFrom((char*)from->indicatorPtr); + SQLLEN * indicatorTo = getAdressBindIndTo((char*)to->indicatorPtr); + + ODBCCONVERT_CHECKNULL_SQLDA; + + SQLGUID *g = (SQLGUID*)getAdressBindDataFrom((char*)from->dataPtr); + + char tmp[37]; + int srcLen = snprintf(tmp, sizeof(tmp), + "%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X", + (unsigned int) g->Data1, g->Data2, g->Data3, + g->Data4[0], g->Data4[1], g->Data4[2], g->Data4[3], + g->Data4[4], g->Data4[5], g->Data4[6], g->Data4[7]); + if ( srcLen < 0 || srcLen > 36 ) srcLen = 36; + + if ( !to->isLocalDataPtr ) + to->allocateLocalDataPtr( srcLen + 1 ); + + memcpy(to->localDataPtr, tmp, srcLen); + to->headSqlVarPtr->setSqlLen((short)srcLen); + to->headSqlVarPtr->setSqlData(to->localDataPtr); + + return SQL_SUCCESS; +} + //////////////////////////////////////////////////////////////////////// // TinyInt diff --git a/OdbcConvert.h b/OdbcConvert.h index 16618b83..a66486cd 100644 --- a/OdbcConvert.h +++ b/OdbcConvert.h @@ -94,6 +94,8 @@ class OdbcConvert // Guid int convGuidToString(DescRecord * from, DescRecord * to); int convGuidToStringW(DescRecord * from, DescRecord * to); + int convGuidToBinary(DescRecord * from, DescRecord * to); + int convGuidToVarString(DescRecord * from, DescRecord * to); // TinyInt int convTinyIntToBoolean(DescRecord * from, DescRecord * to); diff --git a/tests/test_guid_and_binary.cpp b/tests/test_guid_and_binary.cpp index 16f39035..0739596c 100644 --- a/tests/test_guid_and_binary.cpp +++ b/tests/test_guid_and_binary.cpp @@ -467,6 +467,147 @@ TEST_F(Fb4PlusTest, Binary16MapsToGuid) { EXPECT_EQ(ind, 16); } +// ============================================================ +// SQL_C_GUID parameter binding direction (issue #295) +// +// These tests cover SQLBindParameter(SQL_C_GUID, ...) — the input direction. +// They do NOT depend on SQL_GUID column type mapping (T5-5), so they run on +// all Firebird versions that support the underlying SQL types. +// ============================================================ + +class GuidParamBindingTest : public OdbcConnectedTest { +protected: + // The known UUID used by every test — canonical text form and matching + // SQLGUID struct + canonical 16-byte representation. + static constexpr const char* kCanonicalText = + "A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11"; + + static SQLGUID makeKnownGuid() { + SQLGUID g{}; + g.Data1 = 0xA0EEBC99; + g.Data2 = 0x9C0B; + g.Data3 = 0x4EF8; + const unsigned char d4[8] = {0xBB, 0x6D, 0x6B, 0xB9, 0xBD, 0x38, 0x0A, 0x11}; + memcpy(g.Data4, d4, 8); + return g; + } + + int GetServerMajor() { + return GetServerMajorVersion(hDbc); + } +}; + +// Bind SQL_C_GUID to a CHAR(16) CHARACTER SET OCTETS parameter and verify the +// 16 bytes Firebird stores match the canonical UUID byte order. +TEST_F(GuidParamBindingTest, BindGuidToCharOctets16) { + REQUIRE_FIREBIRD_CONNECTION(); + + TempTable table(this, "TEST_PB_GUID_OCT", + "ID INTEGER NOT NULL PRIMARY KEY, " + "VAL CHAR(16) CHARACTER SET OCTETS"); + + SQLGUID guid = makeKnownGuid(); + SQLLEN guidInd = sizeof(guid); + SQLINTEGER id = 1; + SQLLEN idInd = sizeof(id); + + SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_SLONG, SQL_INTEGER, 0, 0, &id, sizeof(id), &idInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLExecDirect(hStmt, + (SQLCHAR*)"INSERT INTO TEST_PB_GUID_OCT (ID, VAL) VALUES (?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + Commit(); + ReallocStmt(); + + // Read back as canonical text via UUID_TO_CHAR — Firebird's UUID_TO_CHAR + // round-trips an OCTETS-string in canonical UUID byte order. + ExecDirect("SELECT UUID_TO_CHAR(VAL) FROM TEST_PB_GUID_OCT WHERE ID = 1"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + SQLCHAR buf[64] = {}; + SQLLEN ind = 0; + ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + std::string text((char*)buf); + while (!text.empty() && text.back() == ' ') text.pop_back(); + EXPECT_EQ(text, kCanonicalText) + << "BINARY(16) GUID round-trip failed: stored bytes do not decode to " + << "the canonical UUID. Got: '" << text << "'"; +} + +// Bind SQL_C_GUID to a VARCHAR parameter (Firebird infers VARCHAR for an +// untyped `?` slot inside CHAR_TO_UUID(?)). The driver must send the 36-char +// canonical UUID string. +TEST_F(GuidParamBindingTest, BindGuidToVarcharViaCharToUuid) { + REQUIRE_FIREBIRD_CONNECTION(); + + SQLGUID guid = makeKnownGuid(); + SQLLEN guidInd = sizeof(guid); + + SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + // Round-trip through CHAR_TO_UUID then UUID_TO_CHAR — exercises the + // SQL_C_GUID → VARCHAR wire path on the way in. + ret = SQLExecDirect(hStmt, + (SQLCHAR*)"SELECT UUID_TO_CHAR(CHAR_TO_UUID(?)) FROM rdb$database", + SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "Exec: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "Fetch: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + SQLCHAR buf[64] = {}; + SQLLEN ind = 0; + ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + std::string text((char*)buf); + while (!text.empty() && text.back() == ' ') text.pop_back(); + EXPECT_EQ(text, kCanonicalText) + << "SQL_C_GUID → VARCHAR round-trip failed. Got: '" << text << "'"; +} + +// Bind SQL_C_GUID to a literal `UUID_TO_CHAR(?)` SELECT — Firebird infers +// BINARY(16) for the parameter slot. The wire payload must be the 16 raw +// bytes of the canonical UUID, not a stringified form. +TEST_F(GuidParamBindingTest, BindGuidToUuidToCharRoundtrip) { + REQUIRE_FIREBIRD_CONNECTION(); + + SQLGUID guid = makeKnownGuid(); + SQLLEN guidInd = sizeof(guid); + + SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLExecDirect(hStmt, + (SQLCHAR*)"SELECT UUID_TO_CHAR(?) FROM rdb$database", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + SQLCHAR buf[64] = {}; + SQLLEN ind = 0; + ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + std::string text((char*)buf); + while (!text.empty() && text.back() == ' ') text.pop_back(); + EXPECT_EQ(text, kCanonicalText) + << "SQL_C_GUID → BINARY(16) (UUID_TO_CHAR(?)) round-trip failed. " + << "Got: '" << text << "' — this is the corruption pattern from issue #295."; +} + // Test: DECFLOAT column insertion and retrieval on Firebird 4+ TEST_F(Fb4PlusTest, DecfloatInsertAndRetrieve) { GTEST_SKIP() << "Requires Phase 8: SQL_GUID type mapping and FB4+ types (not yet merged)";