Skip to content

Commit

Permalink
fix handling the output conn str buff if too small (#194)
Browse files Browse the repository at this point in the history
If the application provides an output buffer for the constructed
connection string, the driver will currently return an error and thus
fail the connection. The correct handling is however to apply truncation
but return the number of characters available without trunctation.

This commit works around swprintf() returning an error code, but
otherwise correctly 0-terminating the destination buffer and not setting
the errno.

(cherry picked from commit ef5dccb)
  • Loading branch information
bpintea committed Nov 13, 2019
1 parent e680250 commit f553b9a
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 80 deletions.
2 changes: 1 addition & 1 deletion driver/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -3206,7 +3206,7 @@ SQLRETURN EsSQLSetConnectAttrW(
RET_HDIAGS(dbc, SQL_STATE_HYC00);

#ifndef NDEBUG
/* MicroStrategy Desktop invoked */
/* MicroStrategy Desktop, Oracle BI invoked */
case 1041:
case 1042:
/* MS Access/Jet proprietary info type */
Expand Down
2 changes: 1 addition & 1 deletion driver/defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
#define ESODBC_MAX_IDENTIFIER_LEN SHRT_MAX
/* "the relationship between the columns in the GROUP BY clause and the
* nonaggregated columns in the select list" */
#define ESODBC_GROUP_BY SQL_GB_NO_RELATION
#define ESODBC_GROUP_BY SQL_GB_NO_RELATION

/* 20 = len("18446744073709551616"), 1 << (sizeof(uint64_t) * 8bits) */
#define ESODBC_PRECISION_UINT64 20
Expand Down
14 changes: 10 additions & 4 deletions driver/dsn.c
Original file line number Diff line number Diff line change
Expand Up @@ -806,13 +806,19 @@ long TEST_API write_connection_string(esodbc_dsn_attrs_st *attrs,
} else {
format = WPFWP_LDESC "=" WPFWP_LDESC ";";
}
errno = 0;
n = swprintf(szConnStrOut + pos, cchConnStrOutMax - pos,
format, LWSTR(iter->kw), LWSTR(iter->val));
/* on buffer too small, swprintf() will 0-terminate it,
* return negative, but not set errno. */
if (n < 0) {
ERRN("failed to outprint connection string (keyword: "
LWPDL ", room: %hd, position: %zu).",
LWSTR(iter->kw), cchConnStrOutMax, pos);
return -1;
if (errno != 0) {
ERRN("failed to print connection string (keyword: "
LWPDL ", room: %hd, position: %zu).",
LWSTR(iter->kw), cchConnStrOutMax, pos);
return -1;
}
assert(szConnStrOut[cchConnStrOutMax - 1] == L'\0');
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions driver/handles.c
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,8 @@ SQLRETURN EsSQLSetEnvAttr(SQLHENV EnvironmentHandle,
// review of the options.
case SQL_OV_ODBC2:
case SQL_OV_ODBC3:
WARNH(EnvironmentHandle, "application version %d not fully"
" supported.", (intptr_t)Value);
case SQL_OV_ODBC3_80:
break;
default:
Expand Down
79 changes: 5 additions & 74 deletions test/connected_dbc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,80 +14,13 @@ extern "C" {

#include "connected_dbc.h"

/*
* Answer ES/SQL sends to SYS TYPES
*/
static const char systypes_answer[] = "\
{\
\"columns\":[\
{\"name\":\"TYPE_NAME\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"DATA_TYPE\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"PRECISION\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"LITERAL_PREFIX\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"LITERAL_SUFFIX\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"CREATE_PARAMS\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"NULLABLE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"CASE_SENSITIVE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"SEARCHABLE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"UNSIGNED_ATTRIBUTE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"FIXED_PREC_SCALE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"AUTO_INCREMENT\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"LOCAL_TYPE_NAME\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"MINIMUM_SCALE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"MAXIMUM_SCALE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"SQL_DATA_TYPE\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"SQL_DATETIME_SUB\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"NUM_PREC_RADIX\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"INTERVAL_PRECISION\",\"type\":\"integer\",\"display_size\":11}\
],\
\"rows\":[\
[\"BYTE\",-6,3,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,-6,0,10,null],\
[\"LONG\",-5,19,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,-5,0,10,null],\
[\"BINARY\",-3,2147483647,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,-3,0,null,null],\
[\"NULL\",0,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,0,0,null,null],\
[\"INTEGER\",4,10,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,4,0,10,null],\
[\"SHORT\",5,5,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,5,0,10,null],\
[\"HALF_FLOAT\",6,3,\"'\",\"'\",null,2,false,3,false,false,false,null,3,3,6,0,2,null],\
[\"FLOAT\",7,7,\"'\",\"'\",null,2,false,3,false,false,false,null,7,7,7,0,2,null],\
[\"DOUBLE\",8,15,\"'\",\"'\",null,2,false,3,false,false,false,null,15,15,8,0,2,null],\
[\"SCALED_FLOAT\",8,15,\"'\",\"'\",null,2,false,3,false,false,false,null,15,15,8,0,2,null],\
[\"KEYWORD\",12,32766,\"'\",\"'\",null,2,true,3,true,false,false,null,null,null,12,0,null,null],\
[\"TEXT\",12,2147483647,\"'\",\"'\",null,2,true,3,true,false,false,null,null,null,12,0,null,null],\
[\"IP\",12,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,12,0,null,null],\
[\"BOOLEAN\",16,1,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,16,0,null,null],\
[\"DATE\",91,29,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,91,1,null,null],\
[\"TIME\",92,18,\"'\",\"'\",null,2,false,3,true,false,false,null,3,3,92,2,null,null],\
[\"DATETIME\",93,29,\"'\",\"'\",null,2,false,3,true,false,false,null,3,3,9,3,null,null],\
[\"INTERVAL_YEAR\",101,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,101,0,null,null],\
[\"INTERVAL_MONTH\",102,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,102,0,null,null],\
[\"INTERVAL_DAY\",103,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,103,0,null,null],\
[\"INTERVAL_HOUR\",104,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,104,0,null,null],\
[\"INTERVAL_MINUTE\",105,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,105,0,null,null],\
[\"INTERVAL_SECOND\",106,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,106,0,null,null],\
[\"INTERVAL_YEAR_TO_MONTH\",107,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,107,0,null,null],\
[\"INTERVAL_DAY_TO_HOUR\",108,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,108,0,null,null],\
[\"INTERVAL_DAY_TO_MINUTE\",109,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,109,0,null,null],\
[\"INTERVAL_DAY_TO_SECOND\",110,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,110,0,null,null],\
[\"INTERVAL_HOUR_TO_MINUTE\",111,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,111,0,null,null],\
[\"INTERVAL_HOUR_TO_SECOND\",112,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,112,0,null,null],\
[\"INTERVAL_MINUTE_TO_SECOND\",113,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,113,0,null,null],\
[\"UNSUPPORTED\",1111,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,1111,0,null,null],\
[\"OBJECT\",2002,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,2002,0,null,null],\
[\"NESTED\",2002,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,2002,0,null,null]\
]\
}";

/* minimal, valid connection string */
static const SQLWCHAR connect_string[] = L"Driver=ElasticODBC";


/*
* Class will provide a "connected" DBC: the ES types are loaded.
*/
ConnectedDBC::ConnectedDBC()
{
SQLRETURN ret;
cstr_st types;
cstr_st types = {0};

assert(getenv("TZ") == NULL);

Expand All @@ -101,14 +34,12 @@ ConnectedDBC::ConnectedDBC()
ret = SQLAllocHandle(SQL_HANDLE_DBC, env, &dbc);
assert(SQL_SUCCEEDED(ret));


types.cnt = sizeof(systypes_answer) - 1;
types.str = (SQLCHAR *)malloc(types.cnt);
types.str = (SQLCHAR *)strdup(SYSTYPES_ANSWER);
assert(types.str != NULL);
memcpy(types.str, systypes_answer, types.cnt);
types.cnt = sizeof(SYSTYPES_ANSWER) - 1;

ret = SQLDriverConnect(dbc, (SQLHWND)&types, (SQLWCHAR *)connect_string,
sizeof(connect_string) / sizeof(connect_string[0]) - 1, NULL, 0, NULL,
ret = SQLDriverConnect(dbc, (SQLHWND)&types, (SQLWCHAR *)CONNECT_STRING,
sizeof(CONNECT_STRING) / sizeof(CONNECT_STRING[0]) - 1, NULL, 0, NULL,
ESODBC_SQL_DRIVER_TEST);
assert(SQL_SUCCEEDED(ret));

Expand Down
65 changes: 65 additions & 0 deletions test/connected_dbc.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,71 @@ extern "C" {
ASSERT_EQ(strncmp((char *)_c1.str, (char *)_c2.str, _c1.cnt), 0); \
} while (0)

/*
* Answer ES/SQL sends to SYS TYPES
*/
#define SYSTYPES_ANSWER "\
{\
\"columns\":[\
{\"name\":\"TYPE_NAME\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"DATA_TYPE\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"PRECISION\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"LITERAL_PREFIX\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"LITERAL_SUFFIX\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"CREATE_PARAMS\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"NULLABLE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"CASE_SENSITIVE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"SEARCHABLE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"UNSIGNED_ATTRIBUTE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"FIXED_PREC_SCALE\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"AUTO_INCREMENT\",\"type\":\"boolean\",\"display_size\":1},\
{\"name\":\"LOCAL_TYPE_NAME\",\"type\":\"keyword\",\"display_size\":32766},\
{\"name\":\"MINIMUM_SCALE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"MAXIMUM_SCALE\",\"type\":\"short\",\"display_size\":6},\
{\"name\":\"SQL_DATA_TYPE\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"SQL_DATETIME_SUB\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"NUM_PREC_RADIX\",\"type\":\"integer\",\"display_size\":11},\
{\"name\":\"INTERVAL_PRECISION\",\"type\":\"integer\",\"display_size\":11}\
],\
\"rows\":[\
[\"BYTE\",-6,3,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,-6,0,10,null],\
[\"LONG\",-5,19,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,-5,0,10,null],\
[\"BINARY\",-3,2147483647,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,-3,0,null,null],\
[\"NULL\",0,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,0,0,null,null],\
[\"INTEGER\",4,10,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,4,0,10,null],\
[\"SHORT\",5,5,\"'\",\"'\",null,2,false,3,false,false,false,null,0,0,5,0,10,null],\
[\"HALF_FLOAT\",6,3,\"'\",\"'\",null,2,false,3,false,false,false,null,3,3,6,0,2,null],\
[\"FLOAT\",7,7,\"'\",\"'\",null,2,false,3,false,false,false,null,7,7,7,0,2,null],\
[\"DOUBLE\",8,15,\"'\",\"'\",null,2,false,3,false,false,false,null,15,15,8,0,2,null],\
[\"SCALED_FLOAT\",8,15,\"'\",\"'\",null,2,false,3,false,false,false,null,15,15,8,0,2,null],\
[\"KEYWORD\",12,32766,\"'\",\"'\",null,2,true,3,true,false,false,null,null,null,12,0,null,null],\
[\"TEXT\",12,2147483647,\"'\",\"'\",null,2,true,3,true,false,false,null,null,null,12,0,null,null],\
[\"IP\",12,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,12,0,null,null],\
[\"BOOLEAN\",16,1,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,16,0,null,null],\
[\"DATE\",91,29,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,91,1,null,null],\
[\"TIME\",92,18,\"'\",\"'\",null,2,false,3,true,false,false,null,3,3,92,2,null,null],\
[\"DATETIME\",93,29,\"'\",\"'\",null,2,false,3,true,false,false,null,3,3,9,3,null,null],\
[\"INTERVAL_YEAR\",101,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,101,0,null,null],\
[\"INTERVAL_MONTH\",102,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,102,0,null,null],\
[\"INTERVAL_DAY\",103,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,103,0,null,null],\
[\"INTERVAL_HOUR\",104,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,104,0,null,null],\
[\"INTERVAL_MINUTE\",105,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,105,0,null,null],\
[\"INTERVAL_SECOND\",106,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,106,0,null,null],\
[\"INTERVAL_YEAR_TO_MONTH\",107,7,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,107,0,null,null],\
[\"INTERVAL_DAY_TO_HOUR\",108,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,108,0,null,null],\
[\"INTERVAL_DAY_TO_MINUTE\",109,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,109,0,null,null],\
[\"INTERVAL_DAY_TO_SECOND\",110,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,110,0,null,null],\
[\"INTERVAL_HOUR_TO_MINUTE\",111,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,111,0,null,null],\
[\"INTERVAL_HOUR_TO_SECOND\",112,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,112,0,null,null],\
[\"INTERVAL_MINUTE_TO_SECOND\",113,23,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,113,0,null,null],\
[\"UNSUPPORTED\",1111,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,1111,0,null,null],\
[\"OBJECT\",2002,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,2002,0,null,null],\
[\"NESTED\",2002,0,\"'\",\"'\",null,2,false,3,true,false,false,null,null,null,2002,0,null,null]\
]\
}"

/* minimal, valid connection string */
#define CONNECT_STRING L"Driver=ElasticODBC"

class ConnectedDBC {
protected:
Expand Down
74 changes: 74 additions & 0 deletions test/test_driverconnect.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

#include <gtest/gtest.h>
#include "connected_dbc.h"

namespace test {

class DriverConnect : public ::testing::Test, public ConnectedDBC
{
protected:
cstr_st types = {0};
SQLHANDLE my_dbc;
SQLSMALLINT out_avail = -1;

void SetUp() override
{
ret = SQLAllocHandle(SQL_HANDLE_DBC, env, &my_dbc);
ASSERT_TRUE(SQL_SUCCEEDED(ret));

types.str = (SQLCHAR *)strdup(SYSTYPES_ANSWER);
ASSERT_TRUE(types.str != NULL);
types.cnt = sizeof(SYSTYPES_ANSWER) - 1;
}

void TearDown() override
{
ret = SQLFreeHandle(SQL_HANDLE_DBC, my_dbc);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
}
};

TEST_F(DriverConnect, OutputCount)
{
ret = SQLDriverConnect(my_dbc, (SQLHWND)&types, (SQLWCHAR *)CONNECT_STRING,
sizeof(CONNECT_STRING) / sizeof(CONNECT_STRING[0]) - 1, NULL, 0,
&out_avail, ESODBC_SQL_DRIVER_TEST);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_TRUE(0 < out_avail);
}

TEST_F(DriverConnect, OutputZeroTerm)
{
static const size_t buff_sz = 1024;
SQLWCHAR out_buff[buff_sz];

ret = SQLDriverConnect(my_dbc, (SQLHWND)&types, (SQLWCHAR *)CONNECT_STRING,
sizeof(CONNECT_STRING) / sizeof(CONNECT_STRING[0]) - 1,
out_buff, buff_sz, &out_avail, ESODBC_SQL_DRIVER_TEST);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_TRUE(out_avail < buff_sz);
ASSERT_EQ(out_buff[out_avail], (SQLWCHAR)L'\0');

}

TEST_F(DriverConnect, OutputTruncated)
{
static const size_t buff_sz = 3;
SQLWCHAR out_buff[buff_sz];

ret = SQLDriverConnect(my_dbc, (SQLHWND)&types, (SQLWCHAR *)CONNECT_STRING,
sizeof(CONNECT_STRING) / sizeof(CONNECT_STRING[0]) - 1,
out_buff, buff_sz, &out_avail, ESODBC_SQL_DRIVER_TEST);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_TRUE(buff_sz < out_avail);
ASSERT_EQ(out_buff[buff_sz - 1], (SQLWCHAR)L'\0');

}

} // test namespace

0 comments on commit f553b9a

Please sign in to comment.