Skip to content

Commit 538a720

Browse files
committed
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.
1 parent ca97f46 commit 538a720

5 files changed

Lines changed: 247 additions & 0 deletions

File tree

IscDbc/Connection.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,8 @@ class HeadSqlVar
524524
//virtual void setSqlSubType ( short subtype ) = 0;
525525
virtual void setSqlLen ( short len ) = 0;
526526
virtual short getSqlMultiple () = 0;
527+
virtual short getSqlSubtype () = 0;
528+
virtual short getSqlLen () = 0;
527529

528530
virtual char * getSqlData() = 0;
529531
virtual short * getSqlInd() = 0;

IscDbc/IscHeadSqlVar.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ class IscHeadSqlVar : public HeadSqlVar
100100
inline void setSqlData ( char* data ) { sqlvar->sqldata = data; }
101101

102102
inline short getSqlMultiple () { return sqlMultiple; }
103+
inline short getSqlSubtype () { return sqlvar->sqlsubtype; }
104+
inline short getSqlLen () { return sqlvar->sqllen; }
103105
inline char * getSqlData() { return sqlvar->sqldata; }
104106
inline short * getSqlInd() { return sqlvar->sqlind; }
105107

OdbcConvert.cpp

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,12 +1001,38 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t
10011001
}
10021002
break;
10031003
case SQL_C_GUID:
1004+
// Wire-side (input parameter binding) — Firebird's IPD describes the slot
1005+
// as either BINARY(16) / CHAR(16) OCTETS, a text VARCHAR/CHAR, or (FB4+)
1006+
// genuine BINARY. Route to dedicated wire-only conversions; leave the
1007+
// pre-existing convGuidToString / convGuidToStringW untouched for the
1008+
// app-side fetch path below.
1009+
if ( to->isIndicatorSqlDa )
1010+
{
1011+
// BINARY(16) / CHAR(16) CHARACTER SET OCTETS — 16 raw bytes
1012+
if ( to->headSqlVarPtr->getSqlSubtype() == 1
1013+
&& to->headSqlVarPtr->getSqlLen() == 16 )
1014+
return &OdbcConvert::convGuidToBinary;
1015+
// FB4+ BINARY/VARBINARY described directly as SQL_C_BINARY
1016+
if ( to->conciseType == SQL_C_BINARY )
1017+
return &OdbcConvert::convGuidToBinary;
1018+
// Text wire (VARCHAR/CHAR, any charset) — 36-char canonical UUID.
1019+
// setTypeText() converts SQL_VARYING to SQL_TEXT so convGuidToVarString
1020+
// can write the bytes without a length prefix.
1021+
to->headSqlVarPtr->setTypeText();
1022+
return &OdbcConvert::convGuidToVarString;
1023+
}
1024+
// App-side output (fetching a column described as SQL_GUID into the
1025+
// application's bound buffer). Currently unreachable because the column
1026+
// side of SQL_GUID mapping is task T5-5 (open), but kept intact for when
1027+
// that lands.
10041028
switch(to->conciseType)
10051029
{
10061030
case SQL_C_CHAR:
10071031
return &OdbcConvert::convGuidToString;
10081032
case SQL_C_WCHAR:
10091033
return &OdbcConvert::convGuidToStringW;
1034+
case SQL_C_BINARY:
1035+
return &OdbcConvert::convGuidToBinary;
10101036
default:
10111037
return &OdbcConvert::notYetImplemented;
10121038
}
@@ -1688,6 +1714,80 @@ int OdbcConvert::convGuidToStringW(DescRecord * from, DescRecord * to)
16881714
return SQL_SUCCESS;
16891715
}
16901716

1717+
// Write SQLGUID as 16 bytes in canonical UUID byte order (Data1/2/3 big-endian, Data4 unchanged).
1718+
// Used when the target is BINARY(16) / CHAR(16) CHARACTER SET OCTETS or SQL_C_BINARY.
1719+
int OdbcConvert::convGuidToBinary(DescRecord * from, DescRecord * to)
1720+
{
1721+
char* pointer = (char*)getAdressBindDataTo((char*)to->dataPtr);
1722+
SQLLEN * indicatorTo = getAdressBindIndTo((char*)to->indicatorPtr);
1723+
SQLLEN * indicatorFrom = getAdressBindIndFrom((char*)from->indicatorPtr);
1724+
1725+
ODBCCONVERT_CHECKNULL( pointer );
1726+
1727+
SQLGUID *g = (SQLGUID*)getAdressBindDataFrom((char*)from->dataPtr);
1728+
1729+
unsigned char buf[16];
1730+
buf[0] = (unsigned char)((g->Data1 >> 24) & 0xFF);
1731+
buf[1] = (unsigned char)((g->Data1 >> 16) & 0xFF);
1732+
buf[2] = (unsigned char)((g->Data1 >> 8) & 0xFF);
1733+
buf[3] = (unsigned char)( g->Data1 & 0xFF);
1734+
buf[4] = (unsigned char)((g->Data2 >> 8) & 0xFF);
1735+
buf[5] = (unsigned char)( g->Data2 & 0xFF);
1736+
buf[6] = (unsigned char)((g->Data3 >> 8) & 0xFF);
1737+
buf[7] = (unsigned char)( g->Data3 & 0xFF);
1738+
memcpy(buf + 8, g->Data4, 8);
1739+
1740+
int outlen = (int)to->length;
1741+
int len = outlen < 16 ? outlen : 16;
1742+
1743+
if ( len > 0 )
1744+
memcpy(pointer, buf, len);
1745+
1746+
if ( to->isIndicatorSqlDa ) {
1747+
to->headSqlVarPtr->setSqlLen(len);
1748+
} else
1749+
if ( indicatorTo )
1750+
setIndicatorPtr(indicatorTo, len, to);
1751+
1752+
return SQL_SUCCESS;
1753+
}
1754+
1755+
// Write SQLGUID as the 36-char canonical UUID string into a Firebird text
1756+
// parameter slot (VARCHAR/CHAR of any text charset). The wire-side SQLDA
1757+
// buffer for an untyped `?` placeholder is sized for VARCHAR(0) — too small
1758+
// for 36 bytes — so we stage the string in the DescRecord's local buffer
1759+
// and redirect Firebird to read from there via setSqlData(), mirroring the
1760+
// idiom transferStringToAllowedType already uses for app→wire string moves.
1761+
// The dispatch in getAdressFunction has already called setTypeText() to
1762+
// convert SQL_VARYING wires to SQL_TEXT (no length prefix), so we just
1763+
// write 36 raw bytes and set sqllen.
1764+
int OdbcConvert::convGuidToVarString(DescRecord * from, DescRecord * to)
1765+
{
1766+
SQLLEN * indicatorFrom = getAdressBindIndFrom((char*)from->indicatorPtr);
1767+
SQLLEN * indicatorTo = getAdressBindIndTo((char*)to->indicatorPtr);
1768+
1769+
ODBCCONVERT_CHECKNULL_SQLDA;
1770+
1771+
SQLGUID *g = (SQLGUID*)getAdressBindDataFrom((char*)from->dataPtr);
1772+
1773+
char tmp[37];
1774+
int srcLen = snprintf(tmp, sizeof(tmp),
1775+
"%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X",
1776+
(unsigned int) g->Data1, g->Data2, g->Data3,
1777+
g->Data4[0], g->Data4[1], g->Data4[2], g->Data4[3],
1778+
g->Data4[4], g->Data4[5], g->Data4[6], g->Data4[7]);
1779+
if ( srcLen < 0 || srcLen > 36 ) srcLen = 36;
1780+
1781+
if ( !to->isLocalDataPtr )
1782+
to->allocateLocalDataPtr( srcLen + 1 );
1783+
1784+
memcpy(to->localDataPtr, tmp, srcLen);
1785+
to->headSqlVarPtr->setSqlLen((short)srcLen);
1786+
to->headSqlVarPtr->setSqlData(to->localDataPtr);
1787+
1788+
return SQL_SUCCESS;
1789+
}
1790+
16911791

16921792
////////////////////////////////////////////////////////////////////////
16931793
// TinyInt

OdbcConvert.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ class OdbcConvert
9494
// Guid
9595
int convGuidToString(DescRecord * from, DescRecord * to);
9696
int convGuidToStringW(DescRecord * from, DescRecord * to);
97+
int convGuidToBinary(DescRecord * from, DescRecord * to);
98+
int convGuidToVarString(DescRecord * from, DescRecord * to);
9799

98100
// TinyInt
99101
int convTinyIntToBoolean(DescRecord * from, DescRecord * to);

tests/test_guid_and_binary.cpp

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,147 @@ TEST_F(Fb4PlusTest, Binary16MapsToGuid) {
467467
EXPECT_EQ(ind, 16);
468468
}
469469

470+
// ============================================================
471+
// SQL_C_GUID parameter binding direction (issue #295)
472+
//
473+
// These tests cover SQLBindParameter(SQL_C_GUID, ...) — the input direction.
474+
// They do NOT depend on SQL_GUID column type mapping (T5-5), so they run on
475+
// all Firebird versions that support the underlying SQL types.
476+
// ============================================================
477+
478+
class GuidParamBindingTest : public OdbcConnectedTest {
479+
protected:
480+
// The known UUID used by every test — canonical text form and matching
481+
// SQLGUID struct + canonical 16-byte representation.
482+
static constexpr const char* kCanonicalText =
483+
"A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11";
484+
485+
static SQLGUID makeKnownGuid() {
486+
SQLGUID g{};
487+
g.Data1 = 0xA0EEBC99;
488+
g.Data2 = 0x9C0B;
489+
g.Data3 = 0x4EF8;
490+
const unsigned char d4[8] = {0xBB, 0x6D, 0x6B, 0xB9, 0xBD, 0x38, 0x0A, 0x11};
491+
memcpy(g.Data4, d4, 8);
492+
return g;
493+
}
494+
495+
int GetServerMajor() {
496+
return GetServerMajorVersion(hDbc);
497+
}
498+
};
499+
500+
// Bind SQL_C_GUID to a CHAR(16) CHARACTER SET OCTETS parameter and verify the
501+
// 16 bytes Firebird stores match the canonical UUID byte order.
502+
TEST_F(GuidParamBindingTest, BindGuidToCharOctets16) {
503+
REQUIRE_FIREBIRD_CONNECTION();
504+
505+
TempTable table(this, "TEST_PB_GUID_OCT",
506+
"ID INTEGER NOT NULL PRIMARY KEY, "
507+
"VAL CHAR(16) CHARACTER SET OCTETS");
508+
509+
SQLGUID guid = makeKnownGuid();
510+
SQLLEN guidInd = sizeof(guid);
511+
SQLINTEGER id = 1;
512+
SQLLEN idInd = sizeof(id);
513+
514+
SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT,
515+
SQL_C_SLONG, SQL_INTEGER, 0, 0, &id, sizeof(id), &idInd);
516+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
517+
ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT,
518+
SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd);
519+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
520+
521+
ret = SQLExecDirect(hStmt,
522+
(SQLCHAR*)"INSERT INTO TEST_PB_GUID_OCT (ID, VAL) VALUES (?, ?)", SQL_NTS);
523+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
524+
Commit();
525+
ReallocStmt();
526+
527+
// Read back as canonical text via UUID_TO_CHAR — Firebird's UUID_TO_CHAR
528+
// round-trips an OCTETS-string in canonical UUID byte order.
529+
ExecDirect("SELECT UUID_TO_CHAR(VAL) FROM TEST_PB_GUID_OCT WHERE ID = 1");
530+
ret = SQLFetch(hStmt);
531+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
532+
533+
SQLCHAR buf[64] = {};
534+
SQLLEN ind = 0;
535+
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
536+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
537+
538+
std::string text((char*)buf);
539+
while (!text.empty() && text.back() == ' ') text.pop_back();
540+
EXPECT_EQ(text, kCanonicalText)
541+
<< "BINARY(16) GUID round-trip failed: stored bytes do not decode to "
542+
<< "the canonical UUID. Got: '" << text << "'";
543+
}
544+
545+
// Bind SQL_C_GUID to a VARCHAR parameter (Firebird infers VARCHAR for an
546+
// untyped `?` slot inside CHAR_TO_UUID(?)). The driver must send the 36-char
547+
// canonical UUID string.
548+
TEST_F(GuidParamBindingTest, BindGuidToVarcharViaCharToUuid) {
549+
REQUIRE_FIREBIRD_CONNECTION();
550+
551+
SQLGUID guid = makeKnownGuid();
552+
SQLLEN guidInd = sizeof(guid);
553+
554+
SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT,
555+
SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd);
556+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
557+
558+
// Round-trip through CHAR_TO_UUID then UUID_TO_CHAR — exercises the
559+
// SQL_C_GUID → VARCHAR wire path on the way in.
560+
ret = SQLExecDirect(hStmt,
561+
(SQLCHAR*)"SELECT UUID_TO_CHAR(CHAR_TO_UUID(?)) FROM rdb$database",
562+
SQL_NTS);
563+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "Exec: " << GetOdbcError(SQL_HANDLE_STMT, hStmt);
564+
565+
ret = SQLFetch(hStmt);
566+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "Fetch: " << GetOdbcError(SQL_HANDLE_STMT, hStmt);
567+
568+
SQLCHAR buf[64] = {};
569+
SQLLEN ind = 0;
570+
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
571+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
572+
573+
std::string text((char*)buf);
574+
while (!text.empty() && text.back() == ' ') text.pop_back();
575+
EXPECT_EQ(text, kCanonicalText)
576+
<< "SQL_C_GUID → VARCHAR round-trip failed. Got: '" << text << "'";
577+
}
578+
579+
// Bind SQL_C_GUID to a literal `UUID_TO_CHAR(?)` SELECT — Firebird infers
580+
// BINARY(16) for the parameter slot. The wire payload must be the 16 raw
581+
// bytes of the canonical UUID, not a stringified form.
582+
TEST_F(GuidParamBindingTest, BindGuidToUuidToCharRoundtrip) {
583+
REQUIRE_FIREBIRD_CONNECTION();
584+
585+
SQLGUID guid = makeKnownGuid();
586+
SQLLEN guidInd = sizeof(guid);
587+
588+
SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT,
589+
SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd);
590+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
591+
592+
ret = SQLExecDirect(hStmt,
593+
(SQLCHAR*)"SELECT UUID_TO_CHAR(?) FROM rdb$database", SQL_NTS);
594+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
595+
596+
ret = SQLFetch(hStmt);
597+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
598+
599+
SQLCHAR buf[64] = {};
600+
SQLLEN ind = 0;
601+
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
602+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
603+
604+
std::string text((char*)buf);
605+
while (!text.empty() && text.back() == ' ') text.pop_back();
606+
EXPECT_EQ(text, kCanonicalText)
607+
<< "SQL_C_GUID → BINARY(16) (UUID_TO_CHAR(?)) round-trip failed. "
608+
<< "Got: '" << text << "' — this is the corruption pattern from issue #295.";
609+
}
610+
470611
// Test: DECFLOAT column insertion and retrieval on Firebird 4+
471612
TEST_F(Fb4PlusTest, DecfloatInsertAndRetrieve) {
472613
GTEST_SKIP() << "Requires Phase 8: SQL_GUID type mapping and FB4+ types (not yet merged)";

0 commit comments

Comments
 (0)