Skip to content

Commit 84babbf

Browse files
committed
guid: cover VARBINARY wire, fix its varying-prefix crash, drop dead arms
Test coverage: - BindGuidToVarcharOctets16 — binds SQL_C_GUID to VARCHAR(16) CHARACTER SET OCTETS (the wire form of native FB4+ VARBINARY(16)). The existing BindGuidToCharOctets16 only covered the fixed CHAR(16) (SQL_TEXT) slot; this exercises the SQL_VARYING (length-prefixed) binary wire, which was previously untested. Bug the new test exposed (and fixes): - For a SQL_VARYING binary wire, convGuidToBinary wrote 16 raw bytes at offset 0 with no 2-byte length prefix. Firebird then read the first GUID bytes as a VARYING length and over-read the buffer — a hard SEGFAULT on the FB6 snapshot (silently tolerated on FB5). The dispatch now calls setTypeText() for the OCTETS subtype-1/len-16 branch, mirroring the text branch: the varying wire becomes fixed SQL_TEXT (OCTETS charset preserved) so the offset-0 write is correct. A fixed CHAR(16) wire is already SQL_TEXT, so this is a no-op there. - The new test is SKIP_ON_FIREBIRD6-guarded: even with the prefix fix, the current FB6 master snapshot aborts this parameterized OCTETS-VARYING insert with a server-side "Stack overflow" — the same parameterized-query incompatibility already guarded across the suite. Exercised on FB 3/4/5. Dead-code removal: - Drop the wire-side `to->conciseType == SQL_C_BINARY` arm. Native FB4+ BINARY/VARBINARY reach the dispatch as subtype-1 / sqllen-16 OCTETS (caught by the branch above); the only type that yields conciseType SQL_C_BINARY is a binary BLOB, not a sane GUID-bind target. - Drop the app-side `case SQL_C_BINARY` arm. That output direction is unreachable until column-side SQL_GUID mapping (T5-5 / #287) lands. convGuidToBinary stays (still used by the OCTETS wire branch). Verified: all 4 GuidParamBindingTest cases pass on FB 5.0.3; on the FB6 snapshot the 3 non-varying cases pass and the VARBINARY case skips; full suite shows no regressions.
1 parent f08e69e commit 84babbf

2 files changed

Lines changed: 71 additions & 11 deletions

File tree

OdbcConvert.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,20 +1001,27 @@ 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.
1004+
// Wire-side (input parameter binding) — Firebird's IPD describes the
1005+
// slot as either (VAR)BINARY(16) / (VAR)CHAR(16) CHARACTER SET OCTETS
1006+
// (subtype 1, sqllen 16) or a text VARCHAR/CHAR. Route to dedicated
1007+
// wire-only conversions; leave the pre-existing convGuidToString /
1008+
// convGuidToStringW untouched for the app-side fetch path below.
10091009
if ( to->isIndicatorSqlDa )
10101010
{
1011-
// BINARY(16) / CHAR(16) CHARACTER SET OCTETS — 16 raw bytes
1011+
// BINARY(16) / VARBINARY(16) — i.e. (VAR)CHAR(16) CHARACTER SET
1012+
// OCTETS, which is how Firebird represents native FB4+ BINARY types
1013+
// on the wire (subtype 1, sqllen 16). Send 16 raw canonical bytes.
1014+
// setTypeText() converts a SQL_VARYING wire (VARBINARY) to SQL_TEXT
1015+
// so convGuidToBinary can write at offset 0 with no length prefix;
1016+
// without it Firebird reads the first GUID bytes as a VARYING length
1017+
// prefix and over-reads the buffer (SEGFAULT on FB6). A fixed
1018+
// CHAR(16) wire is already SQL_TEXT, so this is a no-op there.
10121019
if ( to->headSqlVarPtr->getSqlSubtype() == 1
10131020
&& to->headSqlVarPtr->getSqlLen() == 16 )
1021+
{
1022+
to->headSqlVarPtr->setTypeText();
10141023
return &OdbcConvert::convGuidToBinary;
1015-
// FB4+ BINARY/VARBINARY described directly as SQL_C_BINARY
1016-
if ( to->conciseType == SQL_C_BINARY )
1017-
return &OdbcConvert::convGuidToBinary;
1024+
}
10181025
// Text wire (VARCHAR/CHAR, any charset) — 36-char canonical UUID.
10191026
// setTypeText() converts SQL_VARYING to SQL_TEXT so convGuidToVarString
10201027
// can write the bytes without a length prefix.
@@ -1031,8 +1038,6 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t
10311038
return &OdbcConvert::convGuidToString;
10321039
case SQL_C_WCHAR:
10331040
return &OdbcConvert::convGuidToStringW;
1034-
case SQL_C_BINARY:
1035-
return &OdbcConvert::convGuidToBinary;
10361041
default:
10371042
return &OdbcConvert::notYetImplemented;
10381043
}

tests/test_guid_and_binary.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,61 @@ TEST_F(GuidParamBindingTest, BindGuidToUuidToCharRoundtrip) {
608608
<< "Got: '" << text << "' — this is the corruption pattern from issue #295.";
609609
}
610610

611+
// Bind SQL_C_GUID to a VARCHAR(16) CHARACTER SET OCTETS parameter — the
612+
// wire form of a native FB4+ VARBINARY(16). Unlike CHAR(16) OCTETS (a
613+
// fixed SQL_TEXT slot, covered by BindGuidToCharOctets16), this is a
614+
// SQL_VARYING slot, so it exercises convGuidToBinary writing into a
615+
// length-prefixed wire buffer. The dispatch calls setTypeText() to convert
616+
// the varying wire to SQL_TEXT before writing 16 raw bytes; without that the
617+
// first GUID bytes are read as a VARYING length prefix and the buffer is
618+
// over-read (SEGFAULT on the FB6 snapshot, silent on FB5).
619+
//
620+
// Skipped on FB6: even with the fix, the current FB6 master snapshot aborts
621+
// this parameterized OCTETS-VARYING insert with a server-side "Stack
622+
// overflow" — the same parameterized-query incompatibility already guarded
623+
// across the suite (see SKIP_ON_FIREBIRD6 usages). Exercised on FB 3/4/5.
624+
TEST_F(GuidParamBindingTest, BindGuidToVarcharOctets16) {
625+
REQUIRE_FIREBIRD_CONNECTION();
626+
SKIP_ON_FIREBIRD6();
627+
628+
TempTable table(this, "TEST_PB_GUID_VOCT",
629+
"ID INTEGER NOT NULL PRIMARY KEY, "
630+
"VAL VARCHAR(16) CHARACTER SET OCTETS");
631+
632+
SQLGUID guid = makeKnownGuid();
633+
SQLLEN guidInd = sizeof(guid);
634+
SQLINTEGER id = 1;
635+
SQLLEN idInd = sizeof(id);
636+
637+
SQLRETURN ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT,
638+
SQL_C_SLONG, SQL_INTEGER, 0, 0, &id, sizeof(id), &idInd);
639+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
640+
ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT,
641+
SQL_C_GUID, SQL_GUID, 16, 0, &guid, sizeof(guid), &guidInd);
642+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
643+
644+
ret = SQLExecDirect(hStmt,
645+
(SQLCHAR*)"INSERT INTO TEST_PB_GUID_VOCT (ID, VAL) VALUES (?, ?)", SQL_NTS);
646+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
647+
Commit();
648+
ReallocStmt();
649+
650+
ExecDirect("SELECT UUID_TO_CHAR(VAL) FROM TEST_PB_GUID_VOCT WHERE ID = 1");
651+
ret = SQLFetch(hStmt);
652+
ASSERT_TRUE(SQL_SUCCEEDED(ret)) << GetOdbcError(SQL_HANDLE_STMT, hStmt);
653+
654+
SQLCHAR buf[64] = {};
655+
SQLLEN ind = 0;
656+
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
657+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
658+
659+
std::string text((char*)buf);
660+
while (!text.empty() && text.back() == ' ') text.pop_back();
661+
EXPECT_EQ(text, kCanonicalText)
662+
<< "VARBINARY(16) (VARCHAR(16) OCTETS) GUID round-trip failed. "
663+
<< "Got: '" << text << "'";
664+
}
665+
611666
// Test: DECFLOAT column insertion and retrieval on Firebird 4+
612667
TEST_F(Fb4PlusTest, DecfloatInsertAndRetrieve) {
613668
GTEST_SKIP() << "Requires Phase 8: SQL_GUID type mapping and FB4+ types (not yet merged)";

0 commit comments

Comments
 (0)