fix handling the output conn str buff if too small (#194)

bpintea · web-flow · commit ef5dccb5ebcf · 2019-11-13T13:48:01.000+01:00
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.
diff --git a/driver/connect.c b/driver/connect.c
@@ -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 */
diff --git a/driver/defs.h b/driver/defs.h
@@ -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
diff --git a/driver/dsn.c b/driver/dsn.c
@@ -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');
 					}
 				}
 			}
diff --git a/driver/handles.c b/driver/handles.c
@@ -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:
diff --git a/test/connected_dbc.cc b/test/connected_dbc.cc
@@ -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);
 
@@ -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));
 
diff --git a/test/connected_dbc.h b/test/connected_dbc.h
@@ -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:
diff --git a/test/test_driverconnect.cc b/test/test_driverconnect.cc
@@ -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
+

Original file line number	Diff line number	Diff line change
`@@ -806,13 +806,19 @@ long TEST_API write_connection_string(esodbc_dsn_attrs_st *attrs,`
`806`	`806`	`} else {`
`807`	`807`	`format = WPFWP_LDESC "=" WPFWP_LDESC ";";`
`808`	`808`	`}`
	`809`	`+ errno = 0;`
`809`	`810`	`n = swprintf(szConnStrOut + pos, cchConnStrOutMax - pos,`
`810`	`811`	`format, LWSTR(iter->kw), LWSTR(iter->val));`
	`812`	`+ /* on buffer too small, swprintf() will 0-terminate it,`
	`813`	`+ * return negative, but not set errno. */`
`811`	`814`	`if (n < 0) {`
`812`		`- ERRN("failed to outprint connection string (keyword: "`
`813`		`- LWPDL ", room: %hd, position: %zu).",`
`814`		`- LWSTR(iter->kw), cchConnStrOutMax, pos);`
`815`		`- return -1;`
	`815`	`+ if (errno != 0) {`
	`816`	`+ ERRN("failed to print connection string (keyword: "`
	`817`	`+ LWPDL ", room: %hd, position: %zu).",`
	`818`	`+ LWSTR(iter->kw), cchConnStrOutMax, pos);`
	`819`	`+ return -1;`
	`820`	`+ }`
	`821`	`+ assert(szConnStrOut[cchConnStrOutMax - 1] == L'\0');`
`816`	`822`	`}`
`817`	`823`	`}`
`818`	`824`	`}`