From fc52d66105183e0c76cfb8a64c4d2ffe7341c0b1 Mon Sep 17 00:00:00 2001 From: Lawrin Novitsky Date: Sat, 10 Jun 2023 12:42:29 +0200 Subject: [PATCH] ODBC-392 The fix and the testcase 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. --- ma_connection.c | 23 ++++++++++++++--- ma_platform_posix.c | 10 +++++--- ma_platform_win32.c | 15 +++++++---- ma_string.c | 61 +++++---------------------------------------- ma_string.h | 3 ++- test/basic.c | 25 +++++++++++-------- 6 files changed, 58 insertions(+), 79 deletions(-) diff --git a/ma_connection.c b/ma_connection.c index 94042a70..619566c0 100644 --- a/ma_connection.c +++ b/ma_connection.c @@ -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)) { diff --git a/ma_platform_posix.c b/ma_platform_posix.c index 9895ba28..0e164620 100644 --- a/ma_platform_posix.c +++ b/ma_platform_posix.c @@ -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; @@ -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))) @@ -220,6 +220,8 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Ptr, SQLINTEGER PtrLength, SQLULEN * { --AscLen; } + if (mustBeNullTerminated) + AscStr[AscLen]= '\0'; } else { diff --git a/ma_platform_win32.c b/ma_platform_win32.c index cd5060ad..e46f2337 100644 --- a/ma_platform_win32.c +++ b/ma_platform_win32.c @@ -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; @@ -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; @@ -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; diff --git a/ma_string.c b/ma_string.c index 955ef7fd..328e7eeb 100644 --- a/ma_string.c +++ b/ma_string.c @@ -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) { diff --git a/ma_string.h b/ma_string.h index c3281526..c3f37f31 100644 --- a/ma_string.h +++ b/ma_string.h @@ -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); diff --git a/test/basic.c b/test/basic.c index 51602380..66d79bbb 100644 --- a/test/basic.c +++ b/test/basic.c @@ -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); @@ -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; } @@ -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); @@ -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))) { @@ -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)); { @@ -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 */ @@ -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},