Fix SQL_C_GUID parameter binding sending corrupted data on the wire (#295)

IscDbc/Connection.h

@@ -524,6 +524,13 @@ class HeadSqlVar
 	//virtual void		setSqlSubType ( short subtype ) = 0;
 	virtual void		setSqlLen ( short len ) = 0;
 	virtual short		getSqlMultiple () = 0;
+	virtual short		getSqlLen () = 0;
+
+	// True for Firebird BINARY(n) / VARBINARY(n), i.e. CHAR(n) / VARCHAR(n)
+	// CHARACTER SET OCTETS at the wire layer (the FB4+ standard-compliant
+	// aliases).  Implementation in IscHeadSqlVar.h.
+	virtual bool		isBinary () = 0;
+	virtual bool		isVarBinary () = 0;
 
 	virtual char *		getSqlData() = 0;
 	virtual short *		getSqlInd() = 0;

IscDbc/IscHeadSqlVar.h

@@ -100,9 +100,35 @@ class IscHeadSqlVar : public HeadSqlVar
 	inline void	setSqlData ( char* data ) { sqlvar->sqldata = data; }
 
 	inline short	getSqlMultiple () { return sqlMultiple; }
+	inline short	getSqlLen () { return sqlvar->sqllen; }
 	inline char *	getSqlData() { return sqlvar->sqldata; }
 	inline short *	getSqlInd() { return sqlvar->sqlind; }
 
+	// Charset id for CHARACTER SET OCTETS, used to identify Firebird's
+	// FB4+ BINARY / VARBINARY types at the wire layer.  SqlProperties
+	// carries the charset id in a dedicated `sqlcharset` field (separate
+	// from `sqlsubtype`, which is documented as "BLOBs & Text types only";
+	// see IscDbc/Sqlda.h).  Charset id 1 = OCTETS per the driver's charset
+	// table at IscDbc/MultibyteConvert.cpp and per Firebird's
+	// RDB$CHARACTER_SETS system table.
+	static constexpr unsigned CHARSET_OCTETS = 1;
+
+	// True iff the slot describes a Firebird BINARY(n) — the FB4+ alias for
+	// CHAR(n) CHARACTER SET OCTETS.
+	inline bool		isBinary()
+	{
+		return sqlvar->sqltype == SQL_TEXT
+			&& sqlvar->sqlcharset == CHARSET_OCTETS;
+	}
+
+	// True iff the slot describes a Firebird VARBINARY(n) — the FB4+ alias
+	// for VARCHAR(n) CHARACTER SET OCTETS.
+	inline bool		isVarBinary()
+	{
+		return sqlvar->sqltype == SQL_VARYING
+			&& sqlvar->sqlcharset == CHARSET_OCTETS;
+	}
+
 	// not used
 	//void		setSqlInd( short *ind ) { sqlvar->sqlind = ind; }
 	//void		setSqlType ( short type ) { sqlvar->sqltype = type; }

OdbcConvert.cpp

@@ -1001,14 +1001,50 @@ ADRESS_FUNCTION OdbcConvert::getAdressFunction(DescRecord * from, DescRecord * t
 		}
 		break;
 	case SQL_C_GUID:
-		switch(to->conciseType)
+		// Wire-side (input parameter binding): Firebird's IPD describes the
+		// slot as either BINARY(16) / VARBINARY(16) — i.e. (VAR)CHAR(16)
+		// CHARACTER SET OCTETS, the FB4+ standard-compliant aliases per the
+		// LangRef — or a text (VAR)CHAR. Route to dedicated wire-only
+		// conversions; leave the pre-existing convGuidToString /
+		// convGuidToStringW untouched for the app-side fetch path below.
+		if ( to->isIndicatorSqlDa )
 		{
-		case SQL_C_CHAR:
-			return &OdbcConvert::convGuidToString;
-		case SQL_C_WCHAR:
-			return &OdbcConvert::convGuidToStringW;
-		default:
-			return &OdbcConvert::notYetImplemented;
+			// BINARY(16) or VARBINARY(16). setTypeText() converts a varying
+			// OCTETS wire (VARBINARY) to fixed SQL_TEXT so convGuidToBinary
+			// can write at offset 0 with no VARYING length prefix; OCTETS
+			// charset is preserved.  For a fixed BINARY (SQL_TEXT already)
+			// it's a no-op.  Without this, Firebird reads the first GUID
+			// bytes as a VARYING length prefix and over-reads the buffer
+			// (SEGFAULT on the FB6 snapshot, silent on FB5).
+			if ( to->headSqlVarPtr->getSqlLen() == GUID_BINARY_LEN
+				&& ( to->headSqlVarPtr->isBinary()
+				  || to->headSqlVarPtr->isVarBinary() ) )
+			{
+				to->headSqlVarPtr->setTypeText();
+				return &OdbcConvert::convGuidToBinary;
+			}
+			// Text wire (VARCHAR/CHAR, any non-OCTETS charset) — 36-char
+			// canonical UUID.  setTypeText() converts SQL_VARYING to
+			// SQL_TEXT so convGuidToWireString can write the bytes without
+			// a length prefix.
+			to->headSqlVarPtr->setTypeText();
+			return &OdbcConvert::convGuidToWireString;
+		}
+		else
+		{
+			// App-side output (fetching a column described as SQL_GUID into
+			// the application's bound buffer).  Currently unreachable
+			// because the column-side SQL_GUID mapping is open task T5-5
+			// (#287), but kept intact for when that lands.
+			switch(to->conciseType)
+			{
+			case SQL_C_CHAR:
+				return &OdbcConvert::convGuidToString;
+			case SQL_C_WCHAR:
+				return &OdbcConvert::convGuidToStringW;
+			default:
+				return &OdbcConvert::notYetImplemented;
+			}
 		}
 		break;
 
@@ -1638,6 +1674,21 @@ int OdbcConvert::notYetImplemented(DescRecord * from, DescRecord * to)
 // Guid
 ////////////////////////////////////////////////////////////////////////
 
+// Format a SQLGUID as the 36-char canonical UUID string (8-4-4-4-12 hex,
+// upper-case, no NUL counted).  `out` must hold at least GUID_STRING_LEN + 1
+// bytes.  Returns snprintf's character count — always GUID_STRING_LEN for a
+// well-formed call, or negative on a (practically impossible) encoding error.
+// A GUID never formats to any other length, so callers treat
+// "result != GUID_STRING_LEN" as a hard error rather than clamping.
+static int formatGuidCanonical(char* out, size_t cap, const SQLGUID* g)
+{
+	return snprintf(out, cap,
+		"%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]);
+}
+
 int OdbcConvert::convGuidToString(DescRecord * from, DescRecord * to)
 {
 	char* pointer = (char*)getAdressBindDataTo((char*)to->dataPtr);
@@ -1647,20 +1698,39 @@ int OdbcConvert::convGuidToString(DescRecord * from, DescRecord * to)
 	ODBCCONVERT_CHECKNULL( pointer );
 
 	SQLGUID *g = (SQLGUID*)getAdressBindDataFrom((char*)from->dataPtr);
-	int len, outlen = to->length;
 
-	len = snprintf(pointer, outlen, "%08lX-%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]);
+	char buf[GUID_STRING_LEN + 1];
+	if ( formatGuidCanonical(buf, sizeof(buf), g) != GUID_STRING_LEN )
+	{
+		if ( parentStmt )
+			parentStmt->postError( new OdbcError( 0, "HY000", "Internal error formatting GUID" ) );
+		return SQL_ERROR;
+	}
 
-	if ( len == -1 ) len = outlen;
+	// Copy into the application buffer honoring its size.  ODBC reports the
+	// full (untruncated) length via the indicator and raises 01004 when the
+	// buffer cannot hold all 36 chars + NUL terminator.
+	int outlen = to->length;
+	SQLRETURN ret = SQL_SUCCESS;
+	int copy = GUID_STRING_LEN;
+	if ( outlen <= GUID_STRING_LEN )
+	{
+		copy = outlen > 0 ? outlen - 1 : 0;
+		if ( parentStmt )
+			parentStmt->postError( new OdbcError( 0, "01004", "Data truncated" ) );
+		ret = SQL_SUCCESS_WITH_INFO;
+	}
+	if ( copy > 0 )
+		memcpy( pointer, buf, copy );
+	pointer[copy] = 0;
 
 	if ( to->isIndicatorSqlDa ) {
-		to->headSqlVarPtr->setSqlLen(len);
+		to->headSqlVarPtr->setSqlLen( (short)GUID_STRING_LEN );
 	} else
 	if ( indicatorTo )
-		setIndicatorPtr(indicatorTo, len, to);
+		setIndicatorPtr( indicatorTo, GUID_STRING_LEN, to );
 
-	return SQL_SUCCESS;
+	return ret;
 }
 
 int OdbcConvert::convGuidToStringW(DescRecord * from, DescRecord * to)
@@ -1688,6 +1758,103 @@ 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 wire is BINARY(16) or
+// VARBINARY(16) (i.e. (VAR)CHAR(16) CHARACTER SET OCTETS) — see
+// getAdressFunction dispatch for SQL_C_GUID.
+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[GUID_BINARY_LEN];
+	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);
+
+	// The dispatcher only routes here when sqllen == GUID_BINARY_LEN, so
+	// this defensive check should not fire in practice — but a truncated
+	// GUID is corrupted (not just "shorter"), so signal it as a hard
+	// error rather than silently storing 8 (or whatever) bytes.  ODBC
+	// SQLSTATE 22001 "String data, right truncation".
+	int outlen = (int)to->length;
+	if ( outlen < GUID_BINARY_LEN )
+	{
+		if ( parentStmt )
+			parentStmt->postError( new OdbcError( 0, "22001",
+				"String data, right truncation - GUID requires 16 bytes" ) );
+		return SQL_ERROR;
+	}
+
+	memcpy( pointer, buf, GUID_BINARY_LEN );
+
+	if ( to->isIndicatorSqlDa ) {
+		to->headSqlVarPtr->setSqlLen( GUID_BINARY_LEN );
+	} else
+	if ( indicatorTo )
+		setIndicatorPtr( indicatorTo, GUID_BINARY_LEN, to );
+
+	return SQL_SUCCESS;
+}
+
+// Wire-side counterpart of convGuidToString — writes SQLGUID as the 36-char
+// canonical UUID into a Firebird text parameter slot (VARCHAR/CHAR of any
+// non-OCTETS charset).  Where convGuidToString writes into an app-supplied
+// buffer that the application sized (the SQLGetData fetch path, currently
+// dormant until column-side SQL_GUID mapping lands in T5-5 / #287), this
+// writes into the *wire* slot — and for an untyped `?` placeholder Firebird
+// describes that slot as VARCHAR(0), whose default-sized backing buffer is
+// just 1 byte, nowhere near the 36 we need.
+//
+// So we stage the string in DescRecord::localDataPtr (sized explicitly to
+// GUID_STRING_LEN + 1) and redirect Firebird to read from there via
+// HeadSqlVar::setSqlData() — the same idiom transferStringToAllowedType
+// uses for app→wire string moves.  The dispatch has already called
+// setTypeText() so the wire is SQL_TEXT (no length prefix); we just write
+// 36 raw bytes and set sqllen.
+int OdbcConvert::convGuidToWireString(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[GUID_STRING_LEN + 1];
+	if ( formatGuidCanonical(tmp, sizeof(tmp), g) != GUID_STRING_LEN )
+	{
+		if ( parentStmt )
+			parentStmt->postError( new OdbcError( 0, "HY000", "Internal error formatting GUID" ) );
+		return SQL_ERROR;
+	}
+
+	// Always (re)allocate to guarantee the local buffer holds 36 chars +
+	// NUL.  DescRecord::allocateLocalDataPtr() frees any existing buffer
+	// first, so this is safe to call unconditionally — and necessary,
+	// because a smaller default-sized buffer may already exist (an untyped
+	// `?` described as VARCHAR(0) yields a 1-byte default buffer, which the
+	// 36-byte memcpy would overflow).
+	to->allocateLocalDataPtr( GUID_STRING_LEN + 1 );
+
+	memcpy(to->localDataPtr, tmp, GUID_STRING_LEN);
+	to->headSqlVarPtr->setSqlLen((short)GUID_STRING_LEN);
+	to->headSqlVarPtr->setSqlData(to->localDataPtr);
+
+	return SQL_SUCCESS;
+}
+
 
 ////////////////////////////////////////////////////////////////////////
 // TinyInt

OdbcConvert.h

@@ -48,6 +48,13 @@ class DescRecord;
 class OdbcConvert;
 class OdbcStatement;
 
+// Canonical SQLGUID wire sizes.
+// - GUID_BINARY_LEN: 16 raw bytes, the BINARY(16) / VARBINARY(16) wire form.
+// - GUID_STRING_LEN: 36 chars (8-4-4-4-12 hex+dashes, no NUL terminator),
+//   the canonical text form written into VARCHAR/CHAR slots.
+static constexpr int GUID_BINARY_LEN = 16;
+static constexpr int GUID_STRING_LEN = 36;
+
 typedef int (OdbcConvert::*ADRESS_FUNCTION)(DescRecord * from, DescRecord * to);
 
 class OdbcConvert
@@ -94,6 +101,8 @@ class OdbcConvert
 // Guid
 	int convGuidToString(DescRecord * from, DescRecord * to);
 	int convGuidToStringW(DescRecord * from, DescRecord * to);
+	int convGuidToBinary(DescRecord * from, DescRecord * to);
+	int convGuidToWireString(DescRecord * from, DescRecord * to);
 
 // TinyInt
 	int convTinyIntToBoolean(DescRecord * from, DescRecord * to);