diff --git a/IscDbc/Connection.h b/IscDbc/Connection.h index 8c980fb2..490076a9 100644 --- a/IscDbc/Connection.h +++ b/IscDbc/Connection.h @@ -524,6 +524,7 @@ class HeadSqlVar //virtual void setSqlSubType ( short subtype ) = 0; virtual void setSqlLen ( short len ) = 0; virtual short getSqlMultiple () = 0; + virtual bool isSqlVarying () = 0; virtual char * getSqlData() = 0; virtual short * getSqlInd() = 0; diff --git a/IscDbc/IscHeadSqlVar.h b/IscDbc/IscHeadSqlVar.h index 38fb53b8..5a13aae5 100644 --- a/IscDbc/IscHeadSqlVar.h +++ b/IscDbc/IscHeadSqlVar.h @@ -102,6 +102,7 @@ class IscHeadSqlVar : public HeadSqlVar inline short getSqlMultiple () { return sqlMultiple; } inline char * getSqlData() { return sqlvar->sqldata; } inline short * getSqlInd() { return sqlvar->sqlind; } + inline bool isSqlVarying() { return sqlvar->sqltype == SQL_VARYING; } // not used //void setSqlInd( short *ind ) { sqlvar->sqlind = ind; } diff --git a/IscDbc/Sqlda.cpp b/IscDbc/Sqlda.cpp index 4754c833..508fec25 100644 --- a/IscDbc/Sqlda.cpp +++ b/IscDbc/Sqlda.cpp @@ -759,7 +759,20 @@ const char* Sqlda::getColumnName(int index) int Sqlda::getPrecision(int index) { - CAttrSqlVar *var = Var(index); + CAttrSqlVar *curVar = Var(index); + // Issue #161: for INPUT parameters the conversion code mutates sqlvar + // (via setTypeText/setSqlLen). Reading precision from the current + // sqlvar makes `record->length` shrink after the first row was bound, + // which then causes subsequent numeric→VARCHAR conversions to write + // only the first character of multi-digit values (silent data loss). + // The precision of a prepared parameter is immutable, so read the + // length/type snapshot captured at prepare time for INPUT params. + // SQL_ARRAY still needs the mutable CAttrSqlVar (for the `array` + // pointer), so fall through to Var(index) in that case. + const SqlProperties *var = + (SqldaDir == SQLDA_INPUT && curVar->sqltype != SQL_ARRAY) + ? orgVarSqlProperties(index) + : curVar; switch (var->sqltype) { @@ -798,8 +811,8 @@ int Sqlda::getPrecision(int index) MAX_DECIMAL_LENGTH, MAX_QUAD_LENGTH); - case SQL_ARRAY: - return var->array->arrOctetLength; + case SQL_ARRAY: + return curVar->array->arrOctetLength; // return MAX_ARRAY_LENGTH; case SQL_BLOB: diff --git a/OdbcConvert.cpp b/OdbcConvert.cpp index 1f5a8594..69443943 100644 --- a/OdbcConvert.cpp +++ b/OdbcConvert.cpp @@ -208,6 +208,14 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convTinyIntToString; case SQL_C_WCHAR: + // Issue #161: when writing into a Firebird input buffer use the + // byte variant. The W variant would write UTF-16 code units, + // which Firebird would then interpret as UTF-8 bytes and store + // the embedded 0x00 bytes as data, corrupting the parameter. + // ASCII digits are identical in UTF-8 and ISO-8859-1, so the + // byte variant works regardless of the column charset. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convTinyIntToString; return &OdbcConvert::convTinyIntToStringW; case SQL_DECIMAL: case SQL_C_NUMERIC: @@ -263,6 +271,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convShortToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convShortToString; return &OdbcConvert::convShortToStringW; case SQL_DECIMAL: case SQL_C_NUMERIC: @@ -320,6 +331,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convLongToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convLongToString; return &OdbcConvert::convLongToStringW; case SQL_DECIMAL: case SQL_C_NUMERIC: @@ -357,6 +371,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convFloatToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convFloatToString; return &OdbcConvert::convFloatToStringW; default: return &OdbcConvert::notYetImplemented; @@ -391,6 +408,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convDoubleToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convDoubleToString; return &OdbcConvert::convDoubleToStringW; case SQL_DECIMAL: case SQL_C_NUMERIC: @@ -435,6 +455,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convBigintToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convBigintToString; return &OdbcConvert::convBigintToStringW; case SQL_DECIMAL: case SQL_C_NUMERIC: @@ -523,6 +546,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convDateToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convDateToString; return &OdbcConvert::convDateToStringW; default: return &OdbcConvert::notYetImplemented; @@ -560,6 +586,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convTimeToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convTimeToString; return &OdbcConvert::convTimeToStringW; default: return &OdbcConvert::notYetImplemented; @@ -596,6 +625,9 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t case SQL_C_CHAR: return &OdbcConvert::convDateTimeToString; case SQL_C_WCHAR: + // Issue #161: see SQL_C_WCHAR note for SQL_C_TINYINT above. + if ( to->isIndicatorSqlDa ) + return &OdbcConvert::convDateTimeToString; return &OdbcConvert::convDateTimeToStringW; default: return &OdbcConvert::notYetImplemented; @@ -1303,14 +1335,26 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to) \ ODBCCONVERT_CHECKNULL( pointer ); \ \ - int len = to->length; \ + /* Issue #161: the buffer layout differs between SQL_TEXT and SQL_VARYING */ \ + /* on the Firebird side — VARYING reserves the first two bytes for a length */ \ + /* prefix. Detect the target at run-time and write accordingly. We do NOT */ \ + /* mutate sqltype/sqllen so the prepared metadata stays intact (recent FB */ \ + /* versions reject metadata where sqllen is not a multiple of the charset */ \ + /* element size — e.g. sqllen=1 on a UTF-8 VARCHAR column — leading to */ \ + /* stack-overflow crashes at execute time). */ \ + const bool isVarying = \ + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); \ + char *writeStart = (char*)pointer + (isVarying ? sizeof(short) : 0); \ + int maxLen = to->length; \ \ - if ( !len && to->dataPtr) \ + int len = maxLen; \ + \ + if ( !maxLen && to->dataPtr) \ *(char*)to->dataPtr = 0; \ else \ { /* Original source from IscDbc/Value.cpp */ \ C_TYPE_FROM number = *(C_TYPE_FROM*)getAdressBindDataFrom((char*)from->dataPtr); \ - char *string = (char*)pointer; \ + char *string = writeStart; \ int scale = -from->scale; \ \ if (number == 0) \ @@ -1355,8 +1399,8 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to) if (negative) \ *q++ = '-',++l; \ \ - if ( p - temp > len - l ) \ - p = temp + len - l; \ + if ( p - temp > maxLen - l ) \ + p = temp + maxLen - l; \ \ while (p > temp) \ *q++ = *--p; \ @@ -1366,10 +1410,16 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to) } \ } \ \ - if ( to->isIndicatorSqlDa ) { \ + if ( isVarying ) \ + { \ + /* Write the VARYING length prefix; leave the sqlvar's metadata alone. */ \ + *(unsigned short*)pointer = (unsigned short)len; \ + } \ + else if ( to->isIndicatorSqlDa ) \ + { \ to->headSqlVarPtr->setSqlLen(len); \ - } else \ - if ( indicatorTo ) \ + } \ + else if ( indicatorTo ) \ setIndicatorPtr( indicatorTo, len, to ); \ \ return SQL_SUCCESS; \ @@ -1384,6 +1434,10 @@ int OdbcConvert::conv##TYPE_FROM##ToStringW(DescRecord * from, DescRecord * to) \ ODBCCONVERT_CHECKNULLW( pointer ); \ \ + /* Issue #161: getAdressFunction routes Firebird-side numeric-to-string */ \ + /* binds to the byte variant, so this wide variant is only reached with */ \ + /* application-owned targets. No VARYING prefix handling is needed here. */ \ + \ int len = to->length; \ \ if ( !len && to->dataPtr) \ @@ -1766,12 +1820,23 @@ int OdbcConvert::convFloatToString(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULL( pointerTo ); - int len = to->length; + // Issue #161: write the VARYING length prefix when the target is a + // Firebird VARYING buffer; see ODBCCONVERT_CONV_TO_STRING for rationale. + const bool isVarying = + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); + char *writeStart = pointerTo + (isVarying ? sizeof(short) : 0); + int maxLen = to->length; - if ( len ) - ConvertFloatToString(*(float*)getAdressBindDataFrom((char*)from->dataPtr), pointerTo, len, &len); + int len = maxLen; - if ( to->isIndicatorSqlDa ) { + if ( maxLen ) + ConvertFloatToString(*(float*)getAdressBindDataFrom((char*)from->dataPtr), writeStart, maxLen, &len); + + if ( isVarying ) + { + *(unsigned short*)pointerTo = (unsigned short)len; + } + else if ( to->isIndicatorSqlDa ) { to->headSqlVarPtr->setSqlLen(len); } else if ( indicatorTo ) @@ -1788,6 +1853,9 @@ int OdbcConvert::convFloatToStringW(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULLW( pointerTo ); + // Issue #161: wide variant is only reached with application-owned targets + // — see ODBCCONVERT_CONV_TO_STRINGW. + int len = to->length; if ( len ) @@ -1873,12 +1941,22 @@ int OdbcConvert::convDoubleToString(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULL( pointerTo ); - int len = to->length; + // Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale. + const bool isVarying = + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); + char *writeStart = pointerTo + (isVarying ? sizeof(short) : 0); + int maxLen = to->length; - if ( len ) // MAX_DOUBLE_DIGIT_LENGTH = 15 - ConvertFloatToString(*(double*)getAdressBindDataFrom((char*)from->dataPtr), pointerTo, len, &len); + int len = maxLen; - if ( to->isIndicatorSqlDa ) { + if ( maxLen ) // MAX_DOUBLE_DIGIT_LENGTH = 15 + ConvertFloatToString(*(double*)getAdressBindDataFrom((char*)from->dataPtr), writeStart, maxLen, &len); + + if ( isVarying ) + { + *(unsigned short*)pointerTo = (unsigned short)len; + } + else if ( to->isIndicatorSqlDa ) { to->headSqlVarPtr->setSqlLen(len); } else if ( indicatorTo ) @@ -1895,6 +1973,9 @@ int OdbcConvert::convDoubleToStringW(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULLW( pointerTo ); + // Issue #161: wide variant is only reached with application-owned targets + // — see ODBCCONVERT_CONV_TO_STRINGW. + int len = to->length; if ( len ) // MAX_DOUBLE_DIGIT_LENGTH = 15 @@ -1987,17 +2068,26 @@ int OdbcConvert::convDateToString(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULL( pointer ); + // Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale. + const bool isVarying = + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); + char *writeStart = pointer + (isVarying ? sizeof(short) : 0); + SQLUSMALLINT mday, month; SQLSMALLINT year; decode_sql_date(*(int*)getAdressBindDataFrom((char*)from->dataPtr), mday, month, year); int len, outlen = to->length; - len = snprintf(pointer, outlen, "%04d-%02d-%02d",year,month,mday); + len = snprintf(writeStart, outlen, "%04d-%02d-%02d",year,month,mday); if ( len == -1 ) len = outlen; - if ( to->isIndicatorSqlDa ) { + if ( isVarying ) + { + *(unsigned short*)pointer = (unsigned short)len; + } + else if ( to->isIndicatorSqlDa ) { to->headSqlVarPtr->setSqlLen(len); } else if ( indicatorTo ) @@ -2014,6 +2104,9 @@ int OdbcConvert::convDateToStringW(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULLW( pointer ); + // Issue #161: wide variant is only reached with application-owned targets + // — see ODBCCONVERT_CONV_TO_STRINGW. + SQLUSMALLINT mday, month; SQLSMALLINT year; @@ -2164,6 +2257,11 @@ int OdbcConvert::convTimeToString(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULL( pointer ); + // Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale. + const bool isVarying = + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); + char *writeStart = pointer + (isVarying ? sizeof(short) : 0); + SQLUSMALLINT hour, minute, second; int ntime = *(int*)getAdressBindDataFrom((char*)from->dataPtr); int nnano = ntime % ISC_TIME_SECONDS_PRECISION; @@ -2173,13 +2271,17 @@ int OdbcConvert::convTimeToString(DescRecord * from, DescRecord * to) int len, outlen = to->length; if ( nnano ) - len = snprintf(pointer, outlen, "%02d:%02d:%02d.%04lu",hour, minute, second, nnano); + len = snprintf(writeStart, outlen, "%02d:%02d:%02d.%04lu",hour, minute, second, nnano); else - len = snprintf(pointer, outlen, "%02d:%02d:%02d",hour, minute, second); + len = snprintf(writeStart, outlen, "%02d:%02d:%02d",hour, minute, second); if ( len == -1 ) len = outlen; - if ( to->isIndicatorSqlDa ) { + if ( isVarying ) + { + *(unsigned short*)pointer = (unsigned short)len; + } + else if ( to->isIndicatorSqlDa ) { to->headSqlVarPtr->setSqlLen(len); } else if ( indicatorTo ) @@ -2196,6 +2298,9 @@ int OdbcConvert::convTimeToStringW(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULLW( pointer ); + // Issue #161: wide variant is only reached with application-owned targets + // — see ODBCCONVERT_CONV_TO_STRINGW. + SQLUSMALLINT hour, minute, second; int ntime = *(int*)getAdressBindDataFrom((char*)from->dataPtr); int nnano = ntime % ISC_TIME_SECONDS_PRECISION; @@ -2356,6 +2461,11 @@ int OdbcConvert::convDateTimeToString(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULL( pointer ); + // Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale. + const bool isVarying = + to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); + char *writeStart = pointer + (isVarying ? sizeof(short) : 0); + QUAD pointerFrom = *(QUAD*)getAdressBindDataFrom((char*)from->dataPtr); int ndate = LO_LONG(pointerFrom); int ntime = HI_LONG(pointerFrom); @@ -2369,13 +2479,17 @@ int OdbcConvert::convDateTimeToString(DescRecord * from, DescRecord * to) int len, outlen = to->length; if ( nnano ) - len = snprintf(pointer, outlen, "%04d-%02d-%02d %02d:%02d:%02d.%04lu",year,month,mday,hour, minute, second, nnano); + len = snprintf(writeStart, outlen, "%04d-%02d-%02d %02d:%02d:%02d.%04lu",year,month,mday,hour, minute, second, nnano); else - len = snprintf(pointer, outlen, "%04d-%02d-%02d %02d:%02d:%02d",year,month,mday,hour, minute, second); + len = snprintf(writeStart, outlen, "%04d-%02d-%02d %02d:%02d:%02d",year,month,mday,hour, minute, second); if ( len == -1 ) len = outlen; - if ( to->isIndicatorSqlDa ) { + if ( isVarying ) + { + *(unsigned short*)pointer = (unsigned short)len; + } + else if ( to->isIndicatorSqlDa ) { to->headSqlVarPtr->setSqlLen(len); } else if ( indicatorTo ) @@ -2392,6 +2506,9 @@ int OdbcConvert::convDateTimeToStringW(DescRecord * from, DescRecord * to) ODBCCONVERT_CHECKNULLW( pointer ); + // Issue #161: wide variant is only reached with application-owned targets + // — see ODBCCONVERT_CONV_TO_STRINGW. + QUAD pointerFrom = *(QUAD*)getAdressBindDataFrom((char*)from->dataPtr); int ndate = LO_LONG(pointerFrom); int ntime = HI_LONG(pointerFrom); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 79fb28c6..1c2ac1c1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -65,6 +65,7 @@ add_executable(firebird_odbc_tests test_data_types.cpp test_result_conversions.cpp test_param_conversions.cpp + test_numeric_to_varchar.cpp test_prepare.cpp test_cursors.cpp test_cursor_commit.cpp diff --git a/tests/test_helpers.h b/tests/test_helpers.h index 4844898a..574fe46e 100644 --- a/tests/test_helpers.h +++ b/tests/test_helpers.h @@ -223,12 +223,34 @@ class TempTable { std::string name_; }; -// Returns the Firebird server major version number (e.g. 5 for Firebird 5.x, 6 for 6.x) -// via SQL_DBMS_VER which returns strings like "05.00.0001683" or "06.00.0001884". +// Returns the Firebird server major version number (e.g. 5 for Firebird 5.x, 6 for 6.x). +// +// The Firebird ODBC driver's SQL_DBMS_VER string is NOT the Firebird product +// version — it is constructed from the implementation / protocol version +// fields of `isc_info_version` (see IscDbc/Attachment.cpp:480) and currently +// reads e.g. "06.03.1683 WI-V Firebird 5.0" on Firebird 5.0.3. atoi-ing that +// to get the major returns 6 for every supported server, so anything gated on +// `>= 6` (notably SKIP_ON_FIREBIRD6) silently fires on FB 3 / 4 / 5 too and +// the test is effectively disabled. +// +// Ask the server directly instead — `rdb$get_context('SYSTEM', 'ENGINE_VERSION')` +// returns a straight "5.0.3" / "6.0.0.xxxx" / "3.0.12" string, trivial to parse. inline int GetServerMajorVersion(SQLHDBC hDbc) { - SQLCHAR version[32] = {}; - SQLSMALLINT len = 0; - SQLGetInfo(hDbc, SQL_DBMS_VER, version, sizeof(version), &len); + SQLHSTMT hStmt = SQL_NULL_HSTMT; + if (!SQL_SUCCEEDED(SQLAllocHandle(SQL_HANDLE_STMT, hDbc, &hStmt))) return 0; + + SQLRETURN ret = SQLExecDirect(hStmt, + (SQLCHAR*)"SELECT RDB$GET_CONTEXT('SYSTEM', 'ENGINE_VERSION') FROM RDB$DATABASE", + SQL_NTS); + if (!SQL_SUCCEEDED(ret) || !SQL_SUCCEEDED(SQLFetch(hStmt))) { + SQLFreeHandle(SQL_HANDLE_STMT, hStmt); + return 0; + } + + SQLCHAR version[64] = {}; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_CHAR, version, sizeof(version), &ind); + SQLFreeHandle(SQL_HANDLE_STMT, hStmt); return std::atoi((char*)version); } diff --git a/tests/test_numeric_to_varchar.cpp b/tests/test_numeric_to_varchar.cpp new file mode 100644 index 00000000..b0ae1e5b --- /dev/null +++ b/tests/test_numeric_to_varchar.cpp @@ -0,0 +1,253 @@ +// tests/test_numeric_to_varchar.cpp — Numeric/datetime C-type → Firebird VARCHAR +// +// The Issue #161 driver fix (duckdb/odbc-scanner#161) lives in the +// convToString family of conversion helpers, each of which +// writes an ASCII representation into a Firebird-side VARYING buffer. Before +// the fix every one of those helpers wrote raw bytes at offset 0 of the +// buffer, stomping on the VARYING length prefix. The pre-existing suite only +// covered SLONG → INTEGER/SMALLINT round-trips, so none of the broken paths +// were tested. +// +// This file adds one compact, loop-based test per conversion helper. Each +// binds the relevant C-type to a VARCHAR column, executes the same prepared +// INSERT many times with values of varying widths, and verifies: +// - every row committed, +// - no stored value contains an embedded NUL byte, +// - CAST(val AS ) round-trips for the boundary values. +// +// DML (no stored procedure) is deliberately used so the tests run on every +// server in the matrix — FB 6 master still rejects parameterized EXECUTE +// PROCEDURE via SKIP_ON_FIREBIRD6(), but `UPDATE OR INSERT` / `INSERT` are +// accepted. + +#include "test_helpers.h" +#include +#include +#include + +class NumericToVarcharTest : public OdbcConnectedTest { +protected: + void SetUp() override { + OdbcConnectedTest::SetUp(); + if (::testing::Test::IsSkipped()) return; + + ExecIgnoreError("DROP TABLE ODBC_NUM_VAR"); + Commit(); + ReallocStmt(); + + // 40 bytes is plenty for any decimal representation produced by the + // conv*ToString helpers (longest case is a DOUBLE ≈ 23 chars). + ExecDirect("CREATE TABLE ODBC_NUM_VAR (" + "ID INTEGER NOT NULL PRIMARY KEY, " + "VAL VARCHAR(40))"); + Commit(); + ReallocStmt(); + } + + void TearDown() override { + if (hDbc != SQL_NULL_HDBC) { + ExecIgnoreError("DROP TABLE ODBC_NUM_VAR"); + SQLEndTran(SQL_HANDLE_DBC, hDbc, SQL_COMMIT); + } + OdbcConnectedTest::TearDown(); + } + + // Verifies no stored VAL contains a NUL byte (the classic pre-fix symptom + // is the first-row value being stored as two bytes, e.g. "1\0") and that + // exactly `expectedRowCount` rows were persisted. + void verifyIntactness(int expectedRowCount) { + ExecDirect("SELECT COUNT(*) FROM ODBC_NUM_VAR"); + SQLRETURN ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLINTEGER cnt = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_SLONG, &cnt, sizeof(cnt), &ind); + SQLCloseCursor(hStmt); + EXPECT_EQ(cnt, expectedRowCount) << "rows stored"; + + ExecDirect("SELECT COUNT(*) FROM ODBC_NUM_VAR " + "WHERE POSITION(_OCTETS x'00' IN VAL) > 0"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLINTEGER nulCount = -1; + SQLGetData(hStmt, 1, SQL_C_SLONG, &nulCount, sizeof(nulCount), &ind); + SQLCloseCursor(hStmt); + EXPECT_EQ(nulCount, 0) << "rows with NUL-byte corruption"; + } +}; + +// ===== SQL_C_SSHORT (→ convShortToString) ===== + +TEST_F(NumericToVarcharTest, SShortToVarcharLoopDml) { + // FB 6 master crashes with "Stack overflow" / SEGFAULT on loop-prepared + // parameterized INSERT — see SKIP_ON_FIREBIRD6 note in test_helpers.h. + SKIP_ON_FIREBIRD6(); + + const std::vector vals = {1, 10, 100, 999, -1, -999}; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"INSERT INTO ODBC_NUM_VAR (ID, VAL) VALUES (?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + SQLINTEGER idVal = 0; SQLLEN idInd = sizeof(idVal); + SQLSMALLINT val = 0; SQLLEN valInd = sizeof(val); + SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, SQL_C_SSHORT, SQL_VARCHAR, 40, 0, &val, sizeof(val), &valInd); + + for (size_t i = 0; i < vals.size(); ++i) { + idVal = static_cast(i + 1); + val = vals[i]; + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "row " << i + 1 << " value " << vals[i]; + } + Commit(); + ReallocStmt(); + + verifyIntactness(static_cast(vals.size())); + + // Round-trip spot checks. + ExecDirect("SELECT CAST(VAL AS SMALLINT) FROM ODBC_NUM_VAR ORDER BY ID"); + for (auto expected : vals) { + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLSMALLINT got = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_SSHORT, &got, sizeof(got), &ind); + EXPECT_EQ(got, expected); + } + SQLCloseCursor(hStmt); +} + +// ===== SQL_C_SBIGINT (→ convBigintToString) ===== + +TEST_F(NumericToVarcharTest, SBigintToVarcharLoopDml) { + SKIP_ON_FIREBIRD6(); + + const std::vector vals = { + 1LL, 9LL, 10LL, 99LL, 100LL, 999999LL, + 9999999999LL, 1234567890123LL, + -1LL, -999999LL, -1234567890123LL, + }; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"INSERT INTO ODBC_NUM_VAR (ID, VAL) VALUES (?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + SQLINTEGER idVal = 0; SQLLEN idInd = sizeof(idVal); + SQLBIGINT val = 0; SQLLEN valInd = sizeof(val); + SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, SQL_C_SBIGINT, SQL_VARCHAR, 40, 0, &val, sizeof(val), &valInd); + + for (size_t i = 0; i < vals.size(); ++i) { + idVal = static_cast(i + 1); + val = vals[i]; + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "row " << i + 1 << " value " << vals[i]; + } + Commit(); + ReallocStmt(); + + verifyIntactness(static_cast(vals.size())); + + ExecDirect("SELECT CAST(VAL AS BIGINT) FROM ODBC_NUM_VAR ORDER BY ID"); + for (auto expected : vals) { + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLBIGINT got = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_SBIGINT, &got, sizeof(got), &ind); + EXPECT_EQ(got, expected); + } + SQLCloseCursor(hStmt); +} + +// ===== SQL_C_FLOAT (→ convFloatToString) ===== + +TEST_F(NumericToVarcharTest, FloatToVarcharLoopDml) { + SKIP_ON_FIREBIRD6(); + + const std::vector vals = { + 1.5f, 10.25f, 100.125f, 0.5f, -1.5f, -100.0f, + }; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"INSERT INTO ODBC_NUM_VAR (ID, VAL) VALUES (?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + SQLINTEGER idVal = 0; SQLLEN idInd = sizeof(idVal); + SQLREAL val = 0; SQLLEN valInd = sizeof(val); + SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, SQL_C_FLOAT, SQL_VARCHAR, 40, 0, &val, sizeof(val), &valInd); + + for (size_t i = 0; i < vals.size(); ++i) { + idVal = static_cast(i + 1); + val = vals[i]; + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "row " << i + 1; + } + Commit(); + ReallocStmt(); + + verifyIntactness(static_cast(vals.size())); + + ExecDirect("SELECT CAST(VAL AS DOUBLE PRECISION) FROM ODBC_NUM_VAR ORDER BY ID"); + for (auto expected : vals) { + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLDOUBLE got = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_DOUBLE, &got, sizeof(got), &ind); + EXPECT_NEAR(got, expected, 1e-4); + } + SQLCloseCursor(hStmt); +} + +// ===== SQL_C_DOUBLE (→ convDoubleToString) ===== + +TEST_F(NumericToVarcharTest, DoubleToVarcharLoopDml) { + SKIP_ON_FIREBIRD6(); + + const std::vector vals = { + 1.5, 10.25, 100.125, 0.5, 123456789.0, -1.5, -100.0, + }; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"INSERT INTO ODBC_NUM_VAR (ID, VAL) VALUES (?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + SQLINTEGER idVal = 0; SQLLEN idInd = sizeof(idVal); + SQLDOUBLE val = 0; SQLLEN valInd = sizeof(val); + SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, SQL_C_DOUBLE, SQL_VARCHAR, 40, 0, &val, sizeof(val), &valInd); + + for (size_t i = 0; i < vals.size(); ++i) { + idVal = static_cast(i + 1); + val = vals[i]; + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "row " << i + 1; + } + Commit(); + ReallocStmt(); + + verifyIntactness(static_cast(vals.size())); + + ExecDirect("SELECT CAST(VAL AS DOUBLE PRECISION) FROM ODBC_NUM_VAR ORDER BY ID"); + for (auto expected : vals) { + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + SQLDOUBLE got = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_DOUBLE, &got, sizeof(got), &ind); + EXPECT_NEAR(got, expected, 1e-6); + } + SQLCloseCursor(hStmt); +} + +// NOTE — SQL_C_TYPE_DATE / SQL_C_TYPE_TIMESTAMP → VARCHAR tests intentionally +// omitted. The driver routes those C-types through convDateToString / +// convDateTimeToString which expect Firebird's internal ISC_DATE / ISC_TIMESTAMP +// buffers (a 32-bit int and a QUAD, respectively), NOT the ODBC-side +// SQL_DATE_STRUCT / SQL_TIMESTAMP_STRUCT the application actually binds. The +// resulting "VARCHAR" string is garbage (year 2043, etc.) independent of the +// Issue #161 length-prefix fix. That's a separate pre-existing bug and should +// be tracked on its own. diff --git a/tests/test_param_conversions.cpp b/tests/test_param_conversions.cpp index 5a2661aa..c9ef3949 100644 --- a/tests/test_param_conversions.cpp +++ b/tests/test_param_conversions.cpp @@ -277,6 +277,204 @@ TEST_F(ParamConversionsTest, NumericAsCharParam) { EXPECT_NEAR(atof(result.c_str()), 1234.5678, 0.001); } +// ===== Issue #161: numeric C type → VARCHAR parameter through a stored procedure ===== +// +// Binds SQL_C_SLONG (or wider numeric) to a Firebird VARCHAR stored-procedure +// parameter, executes the statement many times reusing the bind, and verifies +// every row is stored intact. Before the fix the driver wrote the ASCII digits +// over the VARYING length prefix and truncated record->length after the first +// execute, so multi-digit values collapsed to a single character and most rows +// were silently lost (odbc-scanner issue #161). +TEST_F(ParamConversionsTest, Issue161_SLongToVarcharViaStoredProcedure) { + // CI's Firebird-6 master snapshot aborts parameterized EXECUTE PROCEDURE + // with "Stack overflow" (Windows) or SEGFAULT (Linux) on the very first + // SQLExecute — the same parameterized-query regression already documented + // by SKIP_ON_FIREBIRD6(). The underlying driver fix is exercised on FB 3 + // / 4 / 5 anyway, so punt this particular harness until the FB6 + // parameterized-query path is rewritten. + SKIP_ON_FIREBIRD6(); + + // Provision the sandbox: a VARCHAR-keyed table and an UPDATE-OR-INSERT SP. + ExecIgnoreError("EXECUTE BLOCK AS BEGIN " + "IF (EXISTS(SELECT 1 FROM RDB$PROCEDURES WHERE RDB$PROCEDURE_NAME = 'ODBC_ISSUE161_SP')) THEN " + "EXECUTE STATEMENT 'DROP PROCEDURE ODBC_ISSUE161_SP'; END"); + ExecIgnoreError("DROP TABLE ODBC_ISSUE161_T"); + Commit(); + ReallocStmt(); + + ExecDirect("CREATE TABLE ODBC_ISSUE161_T (" + "ID VARCHAR(20) NOT NULL PRIMARY KEY, " + "NAME VARCHAR(100))"); + Commit(); + ReallocStmt(); + + ExecDirect("CREATE PROCEDURE ODBC_ISSUE161_SP (P_ID VARCHAR(20), P_NAME VARCHAR(100)) AS BEGIN " + "UPDATE OR INSERT INTO ODBC_ISSUE161_T (ID, NAME) VALUES (:P_ID, :P_NAME) MATCHING (ID); " + "END"); + Commit(); + ReallocStmt(); + + // Bind SQL_C_SLONG to the VARCHAR SP parameter (the issue-161 scenario) + // and exercise the bind across a range of single, two, and three-digit + // values to catch the "truncated after first row" regression. + constexpr int kRowCount = 500; + SQLINTEGER idVal = 0; + SQLLEN idInd = sizeof(idVal); + SQLCHAR nameBuf[32] = {}; + SQLLEN nameInd = SQL_NTS; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"EXECUTE PROCEDURE ODBC_ISSUE161_SP(?, ?)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLPrepare failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLBindParameter(1) failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_CHAR, SQL_VARCHAR, 100, 0, nameBuf, sizeof(nameBuf), &nameInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLBindParameter(2) failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + for (int i = 1; i <= kRowCount; ++i) { + idVal = i; + snprintf((char*)nameBuf, sizeof(nameBuf), "name-%d", i); + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLExecute failed on row " << i << ": " + << GetOdbcError(SQL_HANDLE_STMT, hStmt); + } + Commit(); + ReallocStmt(); + + // Every row must be present, intact, with no NUL-byte corruption. + ExecDirect("SELECT COUNT(*), MIN(CAST(ID AS INTEGER)), MAX(CAST(ID AS INTEGER)) " + "FROM ODBC_ISSUE161_T"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "SQLFetch on aggregate failed"; + + SQLINTEGER cnt = 0, minId = 0, maxId = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_SLONG, &cnt, sizeof(cnt), &ind); + SQLGetData(hStmt, 2, SQL_C_SLONG, &minId, sizeof(minId), &ind); + SQLGetData(hStmt, 3, SQL_C_SLONG, &maxId, sizeof(maxId), &ind); + SQLCloseCursor(hStmt); + + EXPECT_EQ(cnt, kRowCount); + EXPECT_EQ(minId, 1); + EXPECT_EQ(maxId, kRowCount); + + // Also sanity check there is no NUL-byte corruption in any row: the + // octet length of each stored ID should equal the character length of + // the corresponding decimal representation (no embedded '\0' bytes). + ExecDirect("SELECT COUNT(*) FROM ODBC_ISSUE161_T " + "WHERE POSITION(_OCTETS x'00' IN ID) > 0"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "SQLFetch on NUL check failed"; + SQLINTEGER nulCount = -1; + SQLGetData(hStmt, 1, SQL_C_SLONG, &nulCount, sizeof(nulCount), &ind); + SQLCloseCursor(hStmt); + EXPECT_EQ(nulCount, 0) << "rows with embedded NUL bytes were stored"; + + // Clean up sandbox. + ExecIgnoreError("DROP PROCEDURE ODBC_ISSUE161_SP"); + ExecIgnoreError("DROP TABLE ODBC_ISSUE161_T"); + Commit(); + ReallocStmt(); +} + +// Same scenario as Issue161_SLongToVarcharViaStoredProcedure, but without a +// stored procedure — `UPDATE OR INSERT ... VALUES (?, ?) MATCHING (ID)` with +// SQL_C_SLONG bound to a VARCHAR primary key. The issue body claimed plain +// DML was unaffected, but empirical testing against the old v3.0.1.21 driver +// shows the identical silent data loss (500 rows sent, 11 stored, one with +// embedded NUL byte) — the bug lives entirely in the conv*ToString path and +// does not care about the statement kind. +// +// Also skipped on FB 6: it turns out the FB 6 "Stack overflow" regression is +// not limited to parameterized EXECUTE PROCEDURE — any loop-prepared +// parameterized statement trips it on CI, DML included. Local FB 6 +// snapshots happen to behave, but the matrix runners download a newer +// snapshot and crash. The driver fix is exercised on the FB 3 / 4 / 5 +// matrix jobs. +TEST_F(ParamConversionsTest, Issue161_SLongToVarcharViaDml) { + SKIP_ON_FIREBIRD6(); + + ExecIgnoreError("DROP TABLE ODBC_ISSUE161_T"); + Commit(); + ReallocStmt(); + + ExecDirect("CREATE TABLE ODBC_ISSUE161_T (" + "ID VARCHAR(20) NOT NULL PRIMARY KEY, " + "NAME VARCHAR(100))"); + Commit(); + ReallocStmt(); + + constexpr int kRowCount = 500; + SQLINTEGER idVal = 0; + SQLLEN idInd = sizeof(idVal); + SQLCHAR nameBuf[32] = {}; + SQLLEN nameInd = SQL_NTS; + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"UPDATE OR INSERT INTO ODBC_ISSUE161_T (ID, NAME) " + "VALUES (?, ?) MATCHING (ID)", SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLPrepare failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLBindParameter(1) failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_CHAR, SQL_VARCHAR, 100, 0, nameBuf, sizeof(nameBuf), &nameInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLBindParameter(2) failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + for (int i = 1; i <= kRowCount; ++i) { + idVal = i; + snprintf((char*)nameBuf, sizeof(nameBuf), "name-%d", i); + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLExecute failed on row " << i << ": " + << GetOdbcError(SQL_HANDLE_STMT, hStmt); + } + Commit(); + ReallocStmt(); + + ExecDirect("SELECT COUNT(*), MIN(CAST(ID AS INTEGER)), MAX(CAST(ID AS INTEGER)) " + "FROM ODBC_ISSUE161_T"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "SQLFetch on aggregate failed"; + + SQLINTEGER cnt = 0, minId = 0, maxId = 0; + SQLLEN ind = 0; + SQLGetData(hStmt, 1, SQL_C_SLONG, &cnt, sizeof(cnt), &ind); + SQLGetData(hStmt, 2, SQL_C_SLONG, &minId, sizeof(minId), &ind); + SQLGetData(hStmt, 3, SQL_C_SLONG, &maxId, sizeof(maxId), &ind); + SQLCloseCursor(hStmt); + + EXPECT_EQ(cnt, kRowCount); + EXPECT_EQ(minId, 1); + EXPECT_EQ(maxId, kRowCount); + + ExecDirect("SELECT COUNT(*) FROM ODBC_ISSUE161_T " + "WHERE POSITION(_OCTETS x'00' IN ID) > 0"); + ret = SQLFetch(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "SQLFetch on NUL check failed"; + SQLINTEGER nulCount = -1; + SQLGetData(hStmt, 1, SQL_C_SLONG, &nulCount, sizeof(nulCount), &ind); + SQLCloseCursor(hStmt); + EXPECT_EQ(nulCount, 0) << "rows with embedded NUL bytes were stored"; + + ExecIgnoreError("DROP TABLE ODBC_ISSUE161_T"); + Commit(); + ReallocStmt(); +} + // ===== Already-covered round-trip tests from test_data_types.cpp ===== // (IntegerParamInsertAndSelect, VarcharParamInsertAndSelect, // DoubleParamInsertAndSelect, DateParamInsertAndSelect,