From 053c98f852adb836ffc6cd5708cc898f2b6d560a Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Fri, 22 Feb 2019 11:41:47 +0100 Subject: [PATCH 1/3] fix wide to C string conversion truncation indic. In case of SQLWCHAR to SQLCHAR conversion the truncation indication was wrongly signaled (the conversion was performed correctly, though). This happened because of the wrong number of bytes compared against available buffer: the SQLWCHAR string space instead of the (correct) converted equivalent. --- driver/convert.c | 50 +++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/driver/convert.c b/driver/convert.c index 0608b198..72f9db4e 100644 --- a/driver/convert.c +++ b/driver/convert.c @@ -1382,15 +1382,38 @@ static SQLRETURN wstr_to_cstr(esodbc_rec_st *arec, esodbc_rec_st *irec, gd_offset_apply(stmt, &xstr); + assert(xstr.w.str[xstr.w.cnt] == L'\0'); + /* how much space would the converted string take? */ + in_bytes = WCS2U8(xstr.w.str, (int)xstr.w.cnt + 1, NULL, 0); + if (in_bytes <= 0) { + ERRNH(stmt, "failed to convert wchar* to char* for string `" + LWPDL "`.", LWSTR(&xstr.w)); + RET_HDIAGS(stmt, SQL_STATE_22018); + } + /* out length needs to be provided with no (potential) truncation. */ + if (octet_len_ptr) { + /* chars_0 accounts for 0-terminator, so WCS2U8 will count that in + * the output as well => trim it, since we must not count it when + * indicating the length to the application */ + out_bytes = in_bytes - 1; + write_out_octets(octet_len_ptr, out_bytes, irec); + } else { + DBGH(stmt, "REC@0x%p, NULL octet_len_ptr.", arec); + } + if (data_ptr) { charp = (char *)data_ptr; - in_bytes = (int)buff_octet_size((xstr.w.cnt + 1) * sizeof(SQLWCHAR), - sizeof(SQLCHAR), arec, irec, &state); + /* calculate how much of original data could possibly be copied in + * provided buffer; this will be given as a limitation to W-to-C + * conversion function. */ + in_bytes = (int)buff_octet_size(in_bytes, sizeof(SQLCHAR), arec, irec, + &state); /* trim the original string until it fits in output buffer, with given * length limitation */ for (c = (int)xstr.w.cnt + 1; 0 < c; c --) { out_bytes = WCS2U8(xstr.w.str, c, charp, in_bytes); + /* if user gives 0 as buffer size, out_bytes will also be 0 */ if (out_bytes <= 0) { if (WCS2U8_BUFF_INSUFFICIENT) { continue; @@ -1404,10 +1427,7 @@ static SQLRETURN wstr_to_cstr(esodbc_rec_st *arec, esodbc_rec_st *irec, } } - /* if 0's present => 0 < out_bytes */ - assert(xstr.w.str[xstr.w.cnt] == L'\0'); assert(0 < out_bytes); - /* if user gives 0 as buffer size, out_bytes will also be 0 */ if (charp[out_bytes - 1]) { /* ran out of buffer => not 0-terminated and truncated already */ charp[out_bytes - 1] = 0; @@ -1418,30 +1438,12 @@ static SQLRETURN wstr_to_cstr(esodbc_rec_st *arec, esodbc_rec_st *irec, /* only update offset if data is copied out */ gd_offset_update(stmt, xstr.w.cnt, c); - DBGH(stmt, "REC@0x%p, data_ptr@0x%p, copied %zd bytes: `" LWPD "`.", + DBGH(stmt, "REC@0x%p, data_ptr@0x%p, copied %d bytes: `" LCPD "`.", arec, data_ptr, out_bytes, charp); } else { DBGH(stmt, "REC@0x%p, NULL data_ptr.", arec); } - /* if length needs to be given, calculate it (not truncated) & converted */ - if (octet_len_ptr) { - out_bytes = (size_t)WCS2U8(xstr.w.str, (int)xstr.w.cnt + 1, NULL, 0); - if (out_bytes <= 0) { - ERRNH(stmt, "failed to convert wchar* to char* for string `" - LWPDL "`.", LWSTR(&xstr.w)); - RET_HDIAGS(stmt, SQL_STATE_22018); - } else { - /* chars_0 accounts for 0-terminator, so WCS2U8 will count that in - * the output as well => trim it, since we must not count it when - * indicating the length to the application */ - out_bytes --; - } - write_out_octets(octet_len_ptr, out_bytes, irec); - } else { - DBGH(stmt, "REC@0x%p, NULL octet_len_ptr.", arec); - } - if (state != SQL_STATE_00000) { RET_HDIAGS(stmt, state); } From ea54323eb3401c738e7c057dad0c6add670fef9e Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Fri, 22 Feb 2019 11:47:01 +0100 Subject: [PATCH 2/3] added unit test for the fix --- test/test_conversion_sql2c_string.cc | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/test_conversion_sql2c_string.cc b/test/test_conversion_sql2c_string.cc index 3cf2c686..e0a0abf2 100644 --- a/test/test_conversion_sql2c_string.cc +++ b/test/test_conversion_sql2c_string.cc @@ -45,6 +45,7 @@ TEST_F(ConvertSQL2C_String, String2Char) { ret = SQLFetch(stmt); ASSERT_TRUE(SQL_SUCCEEDED(ret)); + assertState(L"00000"); EXPECT_EQ(ind_len, sizeof(SQL_VAL) - /*\0*/1); EXPECT_STREQ((char/*4gtest*/*)buff, SQL_VAL); @@ -114,6 +115,38 @@ TEST_F(ConvertSQL2C_String, String2Char_zero_copy) { } +TEST_F(ConvertSQL2C_String, String2Char_truncate) { + +#undef SQL_VAL +#undef SQL +#define SQL_VAL "abcdef" +#define SQL "CAST(" SQL_VAL " AS TEXT)" + + const char json_answer[] = "\ +{\ + \"columns\": [\ + {\"name\": \"" SQL "\", \"type\": \"text\"}\ + ],\ + \"rows\": [\ + [\"" SQL_VAL "\"]\ + ]\ +}\ +"; + prepareStatement(json_answer); + + SQLCHAR buff[(sizeof(SQL_VAL) - 1)/2 + 1]; + ret = SQLBindCol(stmt, /*col#*/1, SQL_C_CHAR, &buff, sizeof(buff), &ind_len); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + ret = SQLFetch(stmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + assertState(L"01004"); + + EXPECT_EQ(ind_len, sizeof(SQL_VAL) - /*\0*/1); + EXPECT_STREQ((char/*4gtest*/*)buff, "abc"); +} + + TEST_F(ConvertSQL2C_String, String2SLong) { #undef SQL_RAW From bbb43686deeafa683957431f95c0464165b9c0be Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 25 Feb 2019 23:44:50 +0100 Subject: [PATCH 3/3] fix: consider val 0 on dbl to float range check - double zero will be less than FLT_MIN and still a valid float; - double zero to integers conversions consider this case already and are not affected. --- driver/convert.c | 8 ++++--- driver/queries.c | 2 +- test/test_conversion_sql2c_floats.cc | 33 ++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/driver/convert.c b/driver/convert.c index 72f9db4e..1379bd1b 100644 --- a/driver/convert.c +++ b/driver/convert.c @@ -1332,9 +1332,11 @@ SQLRETURN sql2c_double(esodbc_rec_st *arec, esodbc_rec_st *irec, case SQL_C_FLOAT: REJECT_IF_NULL_DEST_BUFF(stmt, data_ptr); - udbl = dbl < 0 ? -dbl : dbl; - if (udbl < FLT_MIN || FLT_MAX < udbl) { - REJECT_AS_OOR(stmt, dbl, /* is fixed */FALSE, SQLREAL); + if (dbl) { + udbl = dbl < 0 ? -dbl : dbl; + if (udbl < FLT_MIN || FLT_MAX < udbl) { + REJECT_AS_OOR(stmt, dbl, /* is fixed */FALSE, SQLREAL); + } } *(SQLREAL *)data_ptr = (SQLREAL)dbl; write_out_octets(octet_len_ptr, sizeof(SQLREAL), irec); diff --git a/driver/queries.c b/driver/queries.c index c5f5d38f..774accc6 100644 --- a/driver/queries.c +++ b/driver/queries.c @@ -776,7 +776,7 @@ SQLRETURN copy_one_row(esodbc_stmt_st *stmt, SQLULEN pos) case UJT_Long: case UJT_LongLong: ll = UJNumericLongLong(obj); - DBGH(stmt, "value [%zd, %d] is numeric: %lld.", rowno, i + 1, + DBGH(stmt, "value [%zd, %d] is integer: %lld.", rowno, i + 1, ll); ret = sql2c_longlong(arec, irec, pos, ll); break; diff --git a/test/test_conversion_sql2c_floats.cc b/test/test_conversion_sql2c_floats.cc index 6a26e9bb..9a990fe6 100644 --- a/test/test_conversion_sql2c_floats.cc +++ b/test/test_conversion_sql2c_floats.cc @@ -333,6 +333,39 @@ TEST_F(ConvertSQL2C_Floats, Float2Long) { } +TEST_F(ConvertSQL2C_Floats, Double_zero2Float) { + +#undef SQL_RAW +#undef SQL_VAL +#undef SQL +#define SQL_RAW 0.0 +#define SQL_VAL STR(SQL_RAW) +#define SQL "CAST(" SQL_VAL " AS DOUBLE)" + + const char json_answer[] = "\ +{\ + \"columns\": [\ + {\"name\": \"" SQL "\", \"type\": \"double\"}\ + ],\ + \"rows\": [\ + [" SQL_VAL "]\ + ]\ +}\ +"; + prepareStatement(json_answer); + + SQLREAL val; + ret = SQLBindCol(stmt, /*col#*/1, SQL_C_FLOAT, &val, sizeof(val), &ind_len); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + ret = SQLFetch(stmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)); + + EXPECT_EQ(ind_len, sizeof(val)); + EXPECT_LE(SQL_RAW, val); +} + + TEST_F(ConvertSQL2C_Floats, Double2Float) { #undef SQL_RAW