SQLNumParams now counts parameter markers (#174)

This commit adds an simple counter of the parameter markers within an
attached SQL statement.

While a correct and complete implementation would require in our case
sending the statement to the server for parameter analysis, this simple
implementation should work for most cases where the application simply
wants to validate the number of user-provided paramters number against
the number of markers in the statement.

(cherry picked from commit cb5be6f)

Author: bpintea

driver/defs.h

@@ -55,7 +55,7 @@
 #define ESODBC_CATALOG_TERM			"catalog"
 #define ESODBC_TABLE_TERM			"table"
 #define ESODBC_SCHEMA_TERM			"schema"
-#define ESODBC_PARAM_MARKER			"?"
+#define ESODBC_PARAM_MARKER			'?'
 
 /* maximum identifer length: match ES/Lucene byte max */
 #define ESODBC_MAX_IDENTIFIER_LEN		SHRT_MAX

driver/odbc.c

@@ -14,9 +14,6 @@
 #include "catalogue.h"
 #include "tinycbor.h"
 
-//#include "elasticodbc_export.h"
-//#define SQL_API	ELASTICODBC_EXPORT SQL_API
-
 
 #define RET_NOT_IMPLEMENTED(hnd) \
 	do { \

driver/queries.c

@@ -3921,6 +3921,85 @@ SQLRETURN EsSQLColAttributeW(
 #undef PNUMATTR_ASSIGN
 }
 
+/* very simple counter of non-quoted, not-escaped single question marks.
+ * no statement validation done */
+static SQLRETURN count_param_markers(esodbc_stmt_st *stmt, SQLSMALLINT *p_cnt)
+{
+	SQLSMALLINT cnt;
+	wstr_st u16sql;
+	SQLWCHAR *pos, *end, *sav;
+	BOOL quoting, escaping;
+	SQLWCHAR crr, qchar; /* char that starting the quote (`'` or `"`) */
+
+	/* The SQL query is received originally as UTF-16 SQLWCHAR, but converted
+	 * to UTF-8 MB right away and stored like that, for conveience. Easiest to
+	 * parse it is in its original SQLWCHAR format, though and since that's
+	 * only needed (assumably) rarely, we'll convert it back here, instead of
+	 * storing also the original. */
+	if (&u16sql != utf8_to_wstr(&stmt->u8sql, &u16sql)) {
+		ERRH(stmt, "failed to convert stmt `" LCPDL "` back to UTF-16 WC",
+			LCSTR(&stmt->u8sql));
+		RET_HDIAGS(stmt, SQL_STATE_HY000);
+	}
+	pos = u16sql.str;
+	end = pos + u16sql.cnt;
+	cnt = 0;
+	quoting = FALSE;
+	escaping = FALSE;
+	crr = 0;
+	while (pos < end) {
+		switch((crr = *pos ++)) {
+			case MK_WPTR(ESODBC_PARAM_MARKER): /* ~ L'?' */
+				if (escaping || quoting) {
+					break;
+				}
+				/* skip groups `???` */
+				sav = pos - 1; /* position of first `?` */
+				while (pos < end && *pos == crr) {
+					pos ++;
+				}
+				/* only count if single */
+				if (sav + 1 == pos) {
+					cnt ++;
+				}
+				break;
+			case L'"':
+			case L'\'':
+				if (escaping) {
+					break;
+				}
+				if (! quoting) {
+					quoting = TRUE;
+					qchar = crr;
+				} else if (qchar == crr) {
+					quoting = FALSE;
+				} /* else: sequence is: `"..'` or `'.."` -> ignore */
+				break;
+			case MK_WPTR(ESODBC_CHAR_ESCAPE): /* ~ L'\\' */
+				if (escaping) {
+					break;
+				}
+				escaping = TRUE;
+				continue;
+		}
+		escaping = FALSE;
+	}
+
+	if (escaping || quoting) {
+		ERRH(stmt, "invalid SQL statement: [%zu] `" LWPDL "`.", u16sql.cnt,
+			LWSTR(&u16sql));
+		free(u16sql.str);
+		RET_HDIAG(stmt, SQL_STATE_HY000, "failed to parse statement", 0);
+	}
+
+	DBGH(stmt, "counted %hd param marker(s) in SQL `" LWPDL "`.", cnt,
+		LWSTR(&u16sql));
+	free(u16sql.str);
+
+	*p_cnt = cnt;
+	return SQL_SUCCESS;
+}
+
 /* function implementation is correct, but it can't really be used as
  * intended, since the driver's "preparation" doesn't really involve sending
  * it to ES or even parameter marker counting; the later would be doable now,
@@ -3938,8 +4017,16 @@ SQLRETURN EsSQLNumParams(
 		RET_HDIAGS(stmt, SQL_STATE_HY010);
 	}
 
+#	if 0
+	/* The correct implementation, once a statement trully is prepared. */
 	return EsSQLGetDescFieldW(stmt->ipd, NO_REC_NR,
 			SQL_DESC_COUNT, ParameterCountPtr, SQL_IS_SMALLINT, NULL);
+#	else /* 0 */
+	/* Only count params on-demand */
+	/* some apps (like pyodbc) will verify the user-provided parameters number
+	 * against the result of this function -> implement a crude parser. */
+	return count_param_markers(stmt, ParameterCountPtr);
+#	endif /* 0 */
 }
 
 SQLRETURN EsSQLRowCount(_In_ SQLHSTMT StatementHandle, _Out_ SQLLEN *RowCount)

driver/util.c

@@ -588,7 +588,7 @@ cstr_st TEST_API *wstr_to_utf8(wstr_st *src, cstr_st *dst)
 		/* eval the needed space for conversion */
 		len = U16WC_TO_MBU8(src->str, src->cnt, NULL, 0);
 		if (! len) {
-			ERRN("failed to evaluate UTF-8 conversion space necessary for [%zu] "
+			ERRN("failed to eval UTF-8 conversion space necessary for [%zu] "
 				"`" LWPDL "`.", src->cnt, LWSTR(src));
 			return NULL;
 		}
@@ -634,6 +634,65 @@ cstr_st TEST_API *wstr_to_utf8(wstr_st *src, cstr_st *dst)
 	return dst;
 }
 
+wstr_st TEST_API *utf8_to_wstr(cstr_st *src, wstr_st *dst)
+{
+	int len;
+	size_t cnt;
+	void *addr;
+	BOOL nts; /* is the \0 present and counted in source string? */
+
+	if (0 < src->cnt) {
+		nts = !src->str[src->cnt - 1];
+
+		/* eval the needed space for conversion */
+		len = U8MB_TO_U16WC(src->str, src->cnt, NULL, 0);
+		if (! len) {
+			ERRN("failed to eval UTF-16 conversion space necessary for [%zu] "
+				"`" LCPDL "`.", src->cnt, LCSTR(src));
+			return NULL;
+		}
+	} else {
+		nts = FALSE;
+		len = 0;
+	}
+
+	assert(0 <= len);
+	/* explicitely allocate the \0 if not present&counted  */
+	cnt = len + /*0-term?*/!nts;
+	if (! dst) { /* if null destination, allocate that as well */
+		cnt += sizeof(wstr_st);
+	}
+
+	if (! (addr = malloc(cnt * sizeof(SQLWCHAR)))) {
+		ERRN("OOM for size: %zuB.", cnt);
+		return NULL;
+	}
+	if (! dst) {
+		dst = (wstr_st *)addr;
+		dst->str = (SQLWCHAR *)((uint8_t *)addr + sizeof(wstr_st));
+	} else {
+		dst->str = (SQLWCHAR *)addr;
+	}
+
+	if (0 < src->cnt) {
+		/* convert the string */
+		len = U8MB_TO_U16WC(src->str, src->cnt, dst->str, len);
+		if (! len) {
+			/* should not happen, since a first scan already happened */
+			ERRN("failed to UTF-16 convert `" LCPDL "`.", LCSTR(src));
+			free(addr);
+			return NULL;
+		}
+	}
+
+	if (! nts) {
+		dst->str[len] = 0;
+	}
+	dst->cnt = len;
+
+	return dst;
+}
+
 /* Escape `%`, `_`, `\` characters in 'src'.
  * If not 'force'-d, the escaping will stop on detection of pre-existing
  * escaping(*), OR if the chars to be escaped are stand-alone.

driver/util.h

@@ -274,7 +274,7 @@ SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
 	SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp);
 
 /*
- * Converts a wide string to a UTF-8 MB, allocating the necessary space.
+ * Converts a UTF-16 wide string to a UTF-8 MB, allocating the necessary space.
  * The \0 is allocated and written, even if not present in source string, but
  * only counted in output string if counted in input one.
  * If 'dst' is null, the destination is also going to be allocate (collated
@@ -284,6 +284,9 @@ SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
  */
 cstr_st TEST_API *wstr_to_utf8(wstr_st *src, cstr_st *dst);
 
+/* the inverse of wstr_to_utf8(). */
+wstr_st TEST_API *utf8_to_wstr(cstr_st *src, wstr_st *dst);
+
 /* Escape `%`, `_`, `\` characters in 'src'.
  * If not 'force'-d, the escaping will stop on detection of pre-existing
  * escaping(*), OR if the chars to be escaped are stand-alone.

test/connected_dbc.cc

@@ -213,6 +213,12 @@ void ConnectedDBC::prepareStatement()
 	ASSERT_TRUE(SQL_SUCCEEDED(ret));
 }
 
+void ConnectedDBC::prepareStatement(const SQLWCHAR *sql)
+{
+	ret = ATTACH_SQL(stmt, sql, wcslen(sql));
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+}
+
 void ConnectedDBC::prepareStatement(const SQLWCHAR *sql,
     const char *jsonAnswer)
 {

test/connected_dbc.h

@@ -57,6 +57,8 @@ class ConnectedDBC {
 	// use the test name as SQL (for faster logs lookup)
 	void prepareStatement();
 	// use an actual SQL statement (if it might be processed)
+	void prepareStatement(const SQLWCHAR *sql);
+	// use an actual SQL statement (if it might be processed)
 	void prepareStatement(const SQLWCHAR *sql, const char *jsonAnswer);
 	// use test name as SQL and attach given answer
 	void prepareStatement(const char *jsonAnswer);

test/test_queries.cc

@@ -117,6 +117,77 @@ TEST_F(Queries, SQLNativeSql_truncate) {
 	ASSERT_TRUE(wcsncmp(buff, src, sizeof(buff)/sizeof(*buff) - 1) == 0);
 }
 
+TEST_F(Queries, SQLNumParams_no_markers) {
+	prepareStatement(MK_WPTR("SELECT 1"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 0);
+}
+
+TEST_F(Queries, SQLNumParams_markers) {
+	prepareStatement(MK_WPTR("SELECT * FROM foo WHERE bar = ? AND baz = ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 2);
+}
+
+TEST_F(Queries, SQLNumParams_quoted) {
+	prepareStatement(MK_WPTR("foo ' ?' ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 1);
+}
+
+TEST_F(Queries, SQLNumParams_quoted_escaped) {
+	prepareStatement(MK_WPTR("foo ' \\?' ? ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 2);
+}
+
+TEST_F(Queries, SQLNumParams_escaped) {
+	prepareStatement(MK_WPTR("foo  \\? ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 1);
+}
+
+TEST_F(Queries, SQLNumParams_invalid) {
+	prepareStatement(MK_WPTR("foo  '\\? ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_FALSE(SQL_SUCCEEDED(ret));
+}
+
+TEST_F(Queries, SQLNumParams_duplicates) {
+	prepareStatement(MK_WPTR("foo  ??? ?"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 1);
+}
+
+TEST_F(Queries, SQLNumParams_duplicates_escape) {
+	prepareStatement(MK_WPTR("foo ? \\??? ??? ? ??"));
+	SQLRETURN ret;
+	SQLSMALLINT params = -1;
+	ret = SQLNumParams(stmt, &params);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(params, 2);
+}
+
 } // test namespace
 
 /* vim: set noet fenc=utf-8 ff=dos sts=0 sw=4 ts=4 : */

test/test_util.cc

@@ -105,6 +105,24 @@ TEST_F(Util, wstr_to_utf8_no_nts) {
 	free(dst.str);
 }
 
+TEST_F(Util, utf8_to_wstr_unicode) {
+#undef SRC_STR
+#undef SRC_AID
+#define SRC_STR	"XäXüXßX"
+#define SRC_AID	"X\xC3\xA4X\xC3\xBCX\xC3\x9FX"
+	wstr_st src = WSTR_INIT(SRC_STR);
+	cstr_st dst_mb;
+	wstr_st dst_wc;
+
+	ASSERT_EQ(&dst_mb, wstr_to_utf8(&src, &dst_mb));
+	ASSERT_EQ(&dst_wc, utf8_to_wstr(&dst_mb, &dst_wc));
+	ASSERT_STREQ((wchar_t *)src.str, (wchar_t *)dst_wc.str);
+	free(dst_mb.str);
+	free(dst_wc.str);
+}
+
+
+
 TEST_F(Util, ascii_c2w2c)
 {
 #undef SRC_STR