Skip to content

Commit

Permalink
ODBC-392 The fix and the testcase
Browse files Browse the repository at this point in the history 
SQLSetConnectAttr(SQL_ATTR_CURRENT_CATALOG) could work incorrectly if
attribute value length parameter was not SQL_NTS, because the string
could end up not (properly) null terminated after 2 trans-coddings(by DM
to unicode, and by driver to used charset). Using ANSI API is another
pre-condition.
  • Loading branch information
@lawrinn
lawrinn committed Jun 10, 2023
1 parent a210bfa commit fc52d66
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 79 deletions.
23 changes: 19 additions & 4 deletions ma_connection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,27 @@ SQLRETURN MADB_DbcSetAttr(MADB_Dbc *Dbc, SQLINTEGER Attribute, SQLPOINTER ValueP
MADB_FREE(Dbc->CatalogName);
if (isWChar)
{
/* IsAnsi will be set before this, even if it is set before connection */
Dbc->CatalogName= MADB_ConvertFromWChar((SQLWCHAR *)ValuePtr, StringLength, NULL, Dbc->ConnOrSrcCharset, NULL);
/* IsAnsi will be set before this, even if it is set before connection
StringLength from DM here is octets length */
Dbc->CatalogName= MADB_ConvertFromWCharEx((SQLWCHAR *)ValuePtr, StringLength/ sizeof(SQLWCHAR), NULL, Dbc->ConnOrSrcCharset, NULL, TRUE);
}
else
Dbc->CatalogName= _strdup((char *)ValuePtr);

{
if (StringLength == SQL_NTS || *((char*)ValuePtr + StringLength - 1) == '\0')
Dbc->CatalogName= _strdup((char *)ValuePtr);
else
{
if ((Dbc->CatalogName= (char *)MADB_CALLOC(StringLength + 1)))
{
memcpy(Dbc->CatalogName, ValuePtr, StringLength);
Dbc->CatalogName[StringLength]= '\0';
}
}
}
if (Dbc->CatalogName == NULL)
{
MADB_SetError(&Dbc->Error, MADB_ERR_HY001, NULL, 0);
}
if (Dbc->mariadb &&
mysql_select_db(Dbc->mariadb, Dbc->CatalogName))
{
Expand Down
10 changes: 6 additions & 4 deletions ma_platform_posix.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ SQLWCHAR *MADB_ConvertToWchar(const char *Ptr, SQLLEN PtrLength, Client_Charset*
}


/* {{{ MADB_ConvertFromWChar */
char *MADB_ConvertFromWChar(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *Length, Client_Charset *cc,
BOOL *Error)
/* {{{ MADB_ConvertFromWCharEx */
char *MADB_ConvertFromWCharEx(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *Length, Client_Charset *cc,
BOOL *Error, BOOL mustBeNullTerminated)
{
char *AscStr;
size_t AscLen= PtrLength, PtrOctetLen;
Expand Down Expand Up @@ -206,7 +206,7 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *
{
/* PtrLength is in characters. MADB_ConvertString(iconv) needs bytes */
PtrOctetLen= SqlwcsOctetLen(Ptr, &PtrLength);
AscLen= PtrLength*cc->cs_info->char_maxlen;
AscLen= (PtrLength + (mustBeNullTerminated ? 1 : 0))*cc->cs_info->char_maxlen;
}

if (!(AscStr = (char *)MADB_CALLOC(AscLen)))
Expand All @@ -220,6 +220,8 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *
{
--AscLen;
}
if (mustBeNullTerminated)
AscStr[AscLen]= '\0';
}
else
{
Expand Down
15 changes: 10 additions & 5 deletions ma_platform_win32.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,13 @@ SQLWCHAR *MADB_ConvertToWchar(const char *Ptr, SQLLEN PtrLength, Client_Charset*
return WStr;
}

/* {{{ MADB_ConvertFromWChar
Length gets number of written bytes including TN (if WstrCharLen == -1 or SQL_NTS or if WstrCharLen includes
TN in the Wstr) */
char *MADB_ConvertFromWChar(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULEN *Length/*Bytes*/, Client_Charset *cc, BOOL *Error)
/* {{{ MADB_ConvertFromWCharEx
Length gets number of written bytes including TN (if WstrCharLen == -1 or SQL_NTS or if WstrCharLen includes
TN in the Wstr)
@mustBeNullTerminated[in] - forces to result string to be NULL-terminated
*/
char* MADB_ConvertFromWCharEx(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULEN *Length/*Bytes*/, Client_Charset *cc, BOOL *Error,
BOOL mustBeNullTerminated)
{
char *AscStr;
int AscLen, AllocLen;
Expand All @@ -146,6 +149,7 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULE
WstrCharLen= -1;

AllocLen= AscLen= WideCharToMultiByte(cc->CodePage, 0, Wstr, WstrCharLen, NULL, 0, NULL, NULL);
/* WideCharToMultiByte doesn't terminate with NULL unless WstrCharLen is -1 or terminated null is counted in */
if (WstrCharLen != -1)
++AllocLen;

Expand All @@ -155,7 +159,8 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULE
AscLen= WideCharToMultiByte(cc->CodePage, 0, Wstr, WstrCharLen, AscStr, AscLen, NULL, (cc->CodePage != CP_UTF8) ? Error : NULL);
if (AscLen && WstrCharLen == -1)
--AscLen;

if (mustBeNullTerminated)
AscStr[AscLen]= '\0';
if (Length)
*Length= (SQLINTEGER)AscLen;
return AscStr;
Expand Down
61 changes: 6 additions & 55 deletions ma_string.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,63 +390,14 @@ my_bool MADB_DynStrGetValues(MADB_Stmt *Stmt, MADB_DynString *DynString)
return FALSE;
}

char *MADB_GetInsertStatement(MADB_Stmt *Stmt)
/* {{{ MADB_ConvertFromWChar
Length gets number of written bytes including TN (if WstrCharLen == -1 or SQL_NTS or if WstrCharLen includes
TN in the Wstr) */
char* MADB_ConvertFromWChar(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULEN *Length/*Bytes*/, Client_Charset *cc, BOOL *Error)
{
char *StmtStr;
size_t Length= 1024;
char *p;
char *TableName;
unsigned int i;

if (!(StmtStr= MADB_CALLOC(1024)))
{
MADB_SetError(&Stmt->Error, MADB_ERR_HY013, NULL, 0);
return NULL;
}
if (!(TableName= MADB_GetTableName(Stmt)))
goto error;
p= StmtStr;

p+= _snprintf(StmtStr, 1024, "INSERT INTO `%s` (", TableName);
for (i=0; i < mysql_stmt_field_count(Stmt->stmt); i++)
{
if (strlen(StmtStr) > Length - NAME_LEN - 4/* comma + 2 ticks + terminating NULL */)
{
Length+= 1024;
if (!(StmtStr= MADB_REALLOC(StmtStr, Length)))
{
MADB_SetError(&Stmt->Error, MADB_ERR_HY013, NULL, 0);
goto error;
}
}
p+= _snprintf(p, Length - strlen(StmtStr), "%s`%s`", (i==0) ? "" : ",", Stmt->stmt->fields[i].org_name);
}
p+= _snprintf(p, Length - strlen(StmtStr), ") VALUES (");

if (strlen(StmtStr) > Length - mysql_stmt_field_count(Stmt->stmt)*2 - 1)/* , and ? for each column + (- 1 comma for 1st column + closing ')')
+ terminating NULL */
{
Length= strlen(StmtStr) + mysql_stmt_field_count(Stmt->stmt)*2 + 1;
if (!(StmtStr= MADB_REALLOC(StmtStr, Length)))
{
MADB_SetError(&Stmt->Error, MADB_ERR_HY013, NULL, 0);
goto error;
}
}

for (i=0; i < mysql_stmt_field_count(Stmt->stmt); i++)
{
p+= _snprintf(p, Length - strlen(StmtStr), "%s?", (i==0) ? "" : ",");
}
p+= _snprintf(p, Length - strlen(StmtStr), ")");
return StmtStr;

error:
if (StmtStr)
MADB_FREE(StmtStr);
return NULL;
return MADB_ConvertFromWCharEx(Wstr, WstrCharLen, Length, cc, Error, FALSE);
}

/* }}} */

my_bool MADB_ValidateStmt(MADB_QUERY *Query)
{
Expand Down
3 changes: 2 additions & 1 deletion ma_string.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
#ifndef _ma_string_h_
#define _ma_string_h_

char *MADB_ConvertFromWCharEx(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *Length, Client_Charset* cc, BOOL *DefaultCharUsed,
BOOL mustBeNullTerminated);
char *MADB_ConvertFromWChar(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN *Length, Client_Charset* cc, BOOL *DefaultCharUsed);
int MADB_ConvertAnsi2Unicode(Client_Charset* cc, const char *AnsiString, SQLLEN AnsiLength,
SQLWCHAR *UnicodeString, SQLLEN UnicodeLength,
SQLLEN *LengthIndicator, BOOL IsNull, MADB_Error *Error);
char* MADB_GetInsertStatement(MADB_Stmt *Stmt);
char* MADB_GetTableName(MADB_Stmt *Stmt);
char* MADB_GetCatalogName(MADB_Stmt *Stmt);
my_bool MADB_DynStrUpdateSet(MADB_Stmt *Stmt, MADB_DynString *DynString);
Expand Down
25 changes: 15 additions & 10 deletions test/basic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,6 @@ ODBC_TEST(t_describe_nulti)
OK_SIMPLE_STMT(hstmt1, "INSERT INTO t1 VALUES ('test')");
CHECK_STMT_RC(hstmt1, SQLPrepare(hstmt1, (SQLCHAR*)"SELECT * FROM t1", SQL_NTS));


CHECK_STMT_RC(hstmt1, SQLDescribeCol(hstmt1, 1, ColumnName, 64, NULL, 0, 0, 0, 0));

ODBC_Disconnect(henv1, hdbc1, hstmt1);
Expand Down Expand Up @@ -1184,10 +1183,17 @@ ODBC_TEST(t_bug41256)

ODBC_TEST(t_bug44971)
{
/* OK_SIMPLE_STMT(Stmt, "drop database if exists bug44971");
OK_SIMPLE_STMT(Stmt, "create database bug44971");
char buffer[MAX_NAME_LEN];
SQLINTEGER len;

OK_SIMPLE_STMT(Stmt, "DROP DATABASE IF EXISTS bug44971");
OK_SIMPLE_STMT(Stmt, "CREATE DATABASE bug44971");
CHECK_DBC_RC(Connection, SQLGetConnectAttr(Connection, SQL_ATTR_CURRENT_CATALOG, buffer, sizeof(buffer), &len));
CHECK_DBC_RC(Connection, SQLSetConnectAttr(Connection, SQL_ATTR_CURRENT_CATALOG, "bug44971xxx", 8));
OK_SIMPLE_STMT(Stmt, "drop database if exists bug44971");*/
/* Restoring original schema back. This also covers ODBC-392. Somehow it always hits here, but not in the previous line */
CHECK_DBC_RC(Connection, SQLSetConnectAttr(Connection, SQL_ATTR_CURRENT_CATALOG, buffer, len));

OK_SIMPLE_STMT(Stmt, "DROP DATABASE IF EXISTS bug44971");
return OK;
}

Expand All @@ -1200,7 +1206,7 @@ ODBC_TEST(t_bug48603)
HSTMT hstmt1;
SQLCHAR conn[512], conn_out[512], query[53];

OK_SIMPLE_STMT(Stmt, "select @@wait_timeout, @@interactive_timeout");
OK_SIMPLE_STMT(Stmt, "SELECT @@wait_timeout, @@interactive_timeout");
CHECK_STMT_RC(Stmt,SQLFetch(Stmt));

timeout= my_fetch_int(Stmt, 1);
Expand All @@ -1213,7 +1219,7 @@ ODBC_TEST(t_bug48603)
diag("Changing interactive timeout globally as it is equal to wait_timeout");
/* Changing globally interactive timeout to be able to test
if INTERACTIVE option works */
sprintf((char *)query, "set GLOBAL interactive_timeout=%d", timeout + diff);
sprintf((char *)query, "SET GLOBAL interactive_timeout=%d", timeout + diff);

if (!SQL_SUCCEEDED(SQLExecDirect(Stmt, query, SQL_NTS)))
{
Expand Down Expand Up @@ -1249,7 +1255,7 @@ ODBC_TEST(t_bug48603)
SQL_DRIVER_NOPROMPT));

CHECK_DBC_RC(hdbc1, SQLAllocStmt(hdbc1, &hstmt1));
OK_SIMPLE_STMT(hstmt1, "select @@interactive_timeout");
OK_SIMPLE_STMT(hstmt1, "SELECT @@interactive_timeout");
CHECK_STMT_RC(hstmt1,SQLFetch(hstmt1));

{
Expand All @@ -1262,17 +1268,15 @@ ODBC_TEST(t_bug48603)
if (timeout == interactive)
{
/* setting global interactive timeout back if we changed it */
sprintf((char *)query, "set GLOBAL interactive_timeout=%d", timeout);
sprintf((char *)query, "SET GLOBAL interactive_timeout=%d", timeout);
CHECK_STMT_RC(Stmt, SQLExecDirect(Stmt, query, SQL_NTS));
}

is_num(timeout + diff, cur_timeout);
}

return OK;
}


/*
Bug#45378 - spaces in connection string aren't removed
*/
Expand Down Expand Up @@ -1870,6 +1874,7 @@ MA_ODBC_TESTS my_tests[]=
{t_bug28820, "t_bug28820", NORMAL},
{t_bug31959, "t_bug31959", NORMAL},
{t_bug41256, "t_bug41256", NORMAL},
{t_bug44971, "t_bug44971", NORMAL},
{t_bug48603, "t_bug48603", NORMAL},
{t_bug45378, "t_bug45378", NORMAL},
{t_mysqld_stmt_reset, "tmysqld_stmt_reset bug", NORMAL},
Expand Down

0 comments on commit fc52d66

Please sign in to comment.