From 27e32f3f9e29238de895061c1f6b694bceb82688 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Wed, 21 Nov 2018 00:13:21 +0100 Subject: [PATCH 1/5] append 32 suffix to DSN binding library - add 32bit indicative to binding library name, now called esdsnbnd32.dll for x86 builds. --- CMakeLists.txt | 10 +++++----- dsneditor/EsOdbcDsnBinding/EsOdbcDsnBinding.vcxproj | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3d9f5ca4..3d8370e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -313,7 +313,7 @@ add_dependencies(${DRV_NAME} dsneditor) include_directories(${ODBC_INC} ${DRV_SRC_DIR} ${LIBCURL_INC_PATH} ${UJSON4C_INC} ${CTIMESTAMP_PATH_SRC} ${DSNEDITOR_INC_PATH}) target_link_libraries(${DRV_NAME} odbccp32 legacy_stdio_definitions - ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnbnd${CMAKE_IMPORT_LIBRARY_SUFFIX} + ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnbnd${BARCH}${CMAKE_IMPORT_LIBRARY_SUFFIX} libcurl ${LIBCURL_WIN_LIBS}) @@ -351,14 +351,14 @@ install(FILES endif (${LIBCURL_LINK_MODE} MATCHES [Dd][Ll][Ll]) # add editor DLLs install(FILES - ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnedt.dll - ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnbnd.dll + ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnedt${CMAKE_SHARED_LIBRARY_SUFFIX} + ${DSNBND_LIB_BIN_DIR_BASE}-$/esdsnbnd${BARCH}${CMAKE_SHARED_LIBRARY_SUFFIX} DESTINATION ${INSTALL_DIR}) install(TARGETS ${DRV_NAME} DESTINATION ${INSTALL_DIR}) # -# Set up the packaing +# Set up the packaging # set(CPACK_GENERATOR "ZIP") # don't build ALL (but see CMAKE_SKIP_INSTALL_ALL_DEPENDENCY comment) @@ -384,4 +384,4 @@ message("Driver install target: ${INSTALL_DIR}.") message("Driver packaging target: ${CPACK_PACKAGE_FILE_NAME}.") message(" ***** ") -# vim: set noet fenc=utf-8 ff=dos sts=0 sw=4 ts=4 : +# vim: set noet fenc=utf-8 ff=dos sts=0 sw=4 ts=4 tw=78 : diff --git a/dsneditor/EsOdbcDsnBinding/EsOdbcDsnBinding.vcxproj b/dsneditor/EsOdbcDsnBinding/EsOdbcDsnBinding.vcxproj index 4ee180fe..3448be1d 100644 --- a/dsneditor/EsOdbcDsnBinding/EsOdbcDsnBinding.vcxproj +++ b/dsneditor/EsOdbcDsnBinding/EsOdbcDsnBinding.vcxproj @@ -76,7 +76,7 @@ true - esdsnbnd + esdsnbnd32 $(Configuration)\$(Platform)\ $(Configuration)\$(Platform)\ @@ -88,7 +88,7 @@ false - esdsnbnd + esdsnbnd32 $(Configuration)\$(Platform)\ $(Configuration)\$(Platform)\ @@ -192,4 +192,4 @@ - \ No newline at end of file + From cd4f411121524a36fa802fda9f13c718600c65ff Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Wed, 21 Nov 2018 00:23:49 +0100 Subject: [PATCH 2/5] allocate large objects in catalog funcs on heap - all local SQLWCHAR buffers of ESODBC_MAX_IDENTIFIER_LEN count or larger are now allocated, due to the relatively large value of the define (SHRT_MAX). --- driver/catalogue.c | 141 +++++++++++++++++++++++++++++---------------- 1 file changed, 92 insertions(+), 49 deletions(-) diff --git a/driver/catalogue.c b/driver/catalogue.c index 88a85a7c..0a961260 100644 --- a/driver/catalogue.c +++ b/driver/catalogue.c @@ -53,12 +53,19 @@ SQLSMALLINT copy_current_catalog(esodbc_dbc_st *dbc, SQLWCHAR *dest, SQLSMALLINT used = -1; /*failure*/ SQLLEN row_cnt; SQLLEN ind_len = SQL_NULL_DATA; - SQLWCHAR buff[ESODBC_MAX_IDENTIFIER_LEN + /*\0*/1]; + SQLWCHAR *buff; + static const size_t buff_cnt = ESODBC_MAX_IDENTIFIER_LEN + /*\0*/1; wstr_st catalog; + buff = malloc(sizeof(*buff) * buff_cnt); + if (! buff) { + ERRH(dbc, "OOM: %zu wchar_t.", buff_cnt); + return -1; + } + if (! SQL_SUCCEEDED(EsSQLAllocHandle(SQL_HANDLE_STMT, dbc, &stmt))) { ERRH(dbc, "failed to alloc a statement handle."); - return -1; + goto end; } assert(stmt); @@ -88,7 +95,7 @@ SQLSMALLINT copy_current_catalog(esodbc_dbc_st *dbc, SQLWCHAR *dest, } if (! SQL_SUCCEEDED(EsSQLBindCol(stmt, /*col#*/1, SQL_C_WCHAR, buff, - sizeof(buff), &ind_len))) { + sizeof(*buff) * buff_cnt, &ind_len))) { ERRH(dbc, "failed to bind first column."); goto end; } @@ -115,14 +122,17 @@ SQLSMALLINT copy_current_catalog(esodbc_dbc_st *dbc, SQLWCHAR *dest, } end: - /* safe even if no binding occured */ - if (! SQL_SUCCEEDED(EsSQLFreeStmt(stmt, SQL_UNBIND))) { - ERRH(stmt, "failed to unbind statement"); - used = -1; - } - if (! SQL_SUCCEEDED(EsSQLFreeHandle(SQL_HANDLE_STMT, stmt))) { - ERRH(dbc, "failed to free statement handle!"); + if (stmt) { + /* safe even if no binding occured */ + if (! SQL_SUCCEEDED(EsSQLFreeStmt(stmt, SQL_UNBIND))) { + ERRH(stmt, "failed to unbind statement"); + used = -1; + } + if (! SQL_SUCCEEDED(EsSQLFreeHandle(SQL_HANDLE_STMT, stmt))) { + ERRH(dbc, "failed to free statement handle!"); + } } + free(buff); return used; } @@ -189,24 +199,35 @@ SQLRETURN EsSQLTablesW( SQLSMALLINT NameLength4) { esodbc_stmt_st *stmt = STMH(StatementHandle); - SQLRETURN ret; - /* b/c declaring an array with a const doesn't work with MSVC's compiler */ - enum wbuf_len { wbuf_len = sizeof(SQL_TABLES) + SQLRETURN ret = SQL_ERROR;; + enum { + wbuf_cnt = sizeof(SQL_TABLES) + sizeof(SQL_TABLES_CAT) + sizeof(SQL_TABLES_TAB) + sizeof(SQL_TABLES_TYP) - + 3 * ESODBC_MAX_IDENTIFIER_LEN /* it has 4x 0-term space */ + + 3 * ESODBC_MAX_IDENTIFIER_LEN /* it has 4x 0-term space */, + /* 2x: "a,b,c" -> "'a','b','c'" : each "x," => "'x'," */ + tbuf_cnt = 2 * ESODBC_MAX_IDENTIFIER_LEN }; - SQLWCHAR wbuf[wbuf_len]; + SQLWCHAR *wbuf; SQLWCHAR *table, *schema, *catalog, *type; size_t cnt_tab, cnt_sch, cnt_cat, cnt_typ, pos; - /* 2x: "a,b,c" -> "'a','b','c'" : each "x," => "'x'," */ - SQLWCHAR typ_buf[2 * ESODBC_MAX_IDENTIFIER_LEN]; + SQLWCHAR *typ_buf; + void *ptr; if (stmt->metadata_id == SQL_TRUE) { FIXME; // FIXME } + ptr = malloc((wbuf_cnt + tbuf_cnt) * sizeof(SQLWCHAR)); + if (! ptr) { + ERRNH(stmt, "OOM: %zu wbuf_t.", wbuf_cnt + tbuf_cnt); + RET_HDIAGS(stmt, SQL_STATE_HY001); + } else { + wbuf = (SQLWCHAR *)ptr; + typ_buf = &wbuf[wbuf_cnt + 1]; + } + pos = sizeof(SQL_TABLES) - 1; wmemcpy(wbuf, MK_WPTR(SQL_TABLES), pos); @@ -218,17 +239,19 @@ SQLRETURN EsSQLTablesW( ERRH(stmt, "catalog identifier name '" LTPDL "' too long " "(%zd. max=%d).", (int)cnt_cat, catalog, cnt_cat, ESODBC_MAX_IDENTIFIER_LEN); - RET_HDIAG(stmt, SQL_STATE_HY090, "catalog name too long", 0); + SET_HDIAG(stmt, SQL_STATE_HY090, "catalog name too long", 0); + goto end; } } else { cnt_cat = NameLength1; } - cnt_cat = swprintf(wbuf + pos, wbuf_len - pos, SQL_TABLES_CAT, + cnt_cat = swprintf(wbuf + pos, wbuf_cnt - pos, SQL_TABLES_CAT, (int)cnt_cat, catalog); if (cnt_cat <= 0) { ERRH(stmt, "failed to print 'catalog' for tables catalog SQL."); - RET_HDIAGS(stmt, SQL_STATE_HY000); + SET_HDIAG(stmt, SQL_STATE_HY000, "internal printing error", 0); + goto end; } else { pos += cnt_cat; } @@ -242,7 +265,8 @@ SQLRETURN EsSQLTablesW( ERRH(stmt, "schema identifier name '" LTPDL "' too long " "(%zd. max=%d).", (int)cnt_sch, schema, cnt_sch, ESODBC_MAX_IDENTIFIER_LEN); - RET_HDIAG(stmt, SQL_STATE_HY090, "schema name too long", 0); + SET_HDIAG(stmt, SQL_STATE_HY090, "schema name too long", 0); + goto end; } } else { cnt_sch = NameLength2; @@ -251,8 +275,9 @@ SQLRETURN EsSQLTablesW( /* TODO: server support needed for sch. name filtering */ if (wszmemcmp(schema, MK_WPTR(SQL_ALL_SCHEMAS), (long)cnt_sch)) { ERRH(stmt, "filtering by schemas is not supported."); - RET_HDIAG(stmt, SQL_STATE_IM001, "schema filtering not supported", + SET_HDIAG(stmt, SQL_STATE_IM001, "schema filtering not supported", 0); + goto end; } } @@ -265,17 +290,19 @@ SQLRETURN EsSQLTablesW( ERRH(stmt, "table identifier name '" LTPDL "' too long " "(%zd. max=%d).", (int)cnt_tab, table, cnt_tab, ESODBC_MAX_IDENTIFIER_LEN); - RET_HDIAG(stmt, SQL_STATE_HY090, "table name too long", 0); + SET_HDIAG(stmt, SQL_STATE_HY090, "table name too long", 0); + goto end; } } else { cnt_tab = NameLength3; } - cnt_tab = swprintf(wbuf + pos, wbuf_len - pos, SQL_TABLES_TAB, + cnt_tab = swprintf(wbuf + pos, wbuf_cnt - pos, SQL_TABLES_TAB, (int)cnt_tab, table); if (cnt_tab <= 0) { ERRH(stmt, "failed to print 'table' for tables catalog SQL."); - RET_HDIAGS(stmt, SQL_STATE_HY000); + SET_HDIAG(stmt, SQL_STATE_HY000, "internal printing error", 0); + goto end; } else { pos += cnt_tab; } @@ -289,26 +316,33 @@ SQLRETURN EsSQLTablesW( ERRH(stmt, "type identifier name '" LTPDL "' too long " "(%zd. max=%d).", (int)cnt_typ, type, cnt_typ, ESODBC_MAX_IDENTIFIER_LEN); - RET_HDIAG(stmt, SQL_STATE_HY090, "type name too long", 0); + SET_HDIAG(stmt, SQL_STATE_HY090, "type name too long", 0); + goto end; } } else { cnt_typ = NameLength4; } - /* In this argument, "each value can be enclosed in single quotation - * marks (') or unquoted" => quote if not quoted (see GH#30398). */ - if (! wcsnstr(type, cnt_typ, L'\'')) { - cnt_typ = quote_tokens(type, cnt_typ, typ_buf); - type = typ_buf; - } + /* Only print TYPE if non-empty. This is be incorrect, by the book, + * but there's little use to specifying an empty string as the type + * (vs NULL), so this should hopefully be safe. -- Qlik */ + if (0 < cnt_typ) { + /* Here, "each value can be enclosed in single quotation marks (') + * or unquoted" => quote if not quoted (see GH#30398). */ + if (! wcsnstr(type, cnt_typ, L'\'')) { + cnt_typ = quote_tokens(type, cnt_typ, typ_buf); + type = typ_buf; + } - cnt_typ = swprintf(wbuf + pos, wbuf_len - pos, SQL_TABLES_TYP, - (int)cnt_typ, type); - if (cnt_typ <= 0) { - ERRH(stmt, "failed to print 'type' for tables catalog SQL."); - RET_HDIAGS(stmt, SQL_STATE_HY000); - } else { - pos += cnt_typ; + cnt_typ = swprintf(wbuf + pos, wbuf_cnt - pos, SQL_TABLES_TYP, + (int)cnt_typ, type); + if (cnt_typ <= 0) { + ERRH(stmt, "failed to print 'type' for tables catalog SQL."); + SET_HDIAG(stmt, SQL_STATE_HY000, "internal printing error", 0); + goto end; + } else { + pos += cnt_typ; + } } } @@ -320,6 +354,8 @@ SQLRETURN EsSQLTablesW( if (SQL_SUCCEEDED(ret)) { ret = EsSQLExecute(stmt); } +end: + free(ptr); return ret; } @@ -337,9 +373,10 @@ SQLRETURN EsSQLColumnsW ) { esodbc_stmt_st *stmt = STMH(hstmt); - SQLRETURN ret; - SQLWCHAR wbuf[sizeof(SQL_COLUMNS(SQL_COL_CAT)) - + 3 * ESODBC_MAX_IDENTIFIER_LEN]; + SQLRETURN ret = SQL_ERROR; + SQLWCHAR *wbuf; + static const size_t wbuf_cnt = sizeof(SQL_COLUMNS(SQL_COL_CAT)) + + 3 * ESODBC_MAX_IDENTIFIER_LEN; SQLWCHAR *catalog, *schema, *table, *column; size_t cnt_cat, cnt_sch, cnt_tab, cnt_col, pos; @@ -428,19 +465,23 @@ SQLRETURN EsSQLColumnsW cnt_col = sizeof(ESODBC_ALL_COLUMNS) - /*0-term*/1; } + wbuf = malloc(wbuf_cnt * sizeof(*wbuf)); + if (! wbuf) { + ERRNH(stmt, "OOM: %zu wbuf_t.", wbuf_cnt); + RET_HDIAGS(stmt, SQL_STATE_HY001); + } /* print SQL to send to server */ if (catalog) { - pos = swprintf(wbuf, sizeof(wbuf)/sizeof(wbuf[0]), - SQL_COLUMNS(SQL_COL_CAT), (int)cnt_cat, catalog, - (int)cnt_tab, table, (int)cnt_col, column); + pos = swprintf(wbuf, wbuf_cnt, SQL_COLUMNS(SQL_COL_CAT), (int)cnt_cat, + catalog, (int)cnt_tab, table, (int)cnt_col, column); } else { - pos = swprintf(wbuf, sizeof(wbuf)/sizeof(wbuf[0]), - SQL_COLUMNS(), - (int)cnt_tab, table, (int)cnt_col, column); + pos = swprintf(wbuf, wbuf_cnt, SQL_COLUMNS(), (int)cnt_tab, table, + (int)cnt_col, column); } if (pos <= 0) { ERRH(stmt, "failed to print 'columns' catalog SQL."); - RET_HDIAGS(stmt, SQL_STATE_HY000); + SET_HDIAG(stmt, SQL_STATE_HY000, "internal printing error", 0); + goto end; } ret = EsSQLFreeStmt(stmt, ESODBC_SQL_CLOSE); @@ -449,6 +490,8 @@ SQLRETURN EsSQLColumnsW if (SQL_SUCCEEDED(ret)) { ret = EsSQLExecute(stmt); } +end: + free(wbuf); return ret; } From 8af26494c05e4b1e2d4250f3fbecb529205aa9d2 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Wed, 21 Nov 2018 00:28:44 +0100 Subject: [PATCH 3/5] b/f: only use _LARGE body size option on x64 - libcurl expects a 64bit value with _LARGE attribute, which is not supplied in x86 builds. --- driver/connect.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/driver/connect.c b/driver/connect.c index c5aa537d..90a3403b 100644 --- a/driver/connect.c +++ b/driver/connect.c @@ -492,9 +492,21 @@ static BOOL dbc_curl_prepare(esodbc_dbc_st *dbc, SQLULEN tout, } } + /* curl will always expect a 64bit value (curl_off_t), while size_t + * remains a 32bit type of 32bit platforms (where a long is needed). */ +#if defined(_WIN64) || defined (WIN64) /* len of the body */ dbc->curl_err = curl_easy_setopt(dbc->curl, CURLOPT_POSTFIELDSIZE_LARGE, u8body->cnt); +#else /* 64b */ + if (LONG_MAX < u8body->cnt) { + ERRH(dbc, "libcurl: post fieldsize %zu too large; max: %ld.", + u8body->cnt, LONG_MAX); + goto err; + } + dbc->curl_err = curl_easy_setopt(dbc->curl, CURLOPT_POSTFIELDSIZE, + u8body->cnt); +#endif /* 64b */ if (dbc->curl_err != CURLE_OK) { ERRH(dbc, "libcurl: failed to set post fieldsize: %zu.", u8body->cnt); goto err; From e730d9b6942119300f3a96766ec0695bac96f0f8 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Wed, 21 Nov 2018 00:35:40 +0100 Subject: [PATCH 4/5] fix: 32 bit compilation warnings - on signed/unsigned comparisons. --- driver/connect.c | 4 ++-- driver/log.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/driver/connect.c b/driver/connect.c index 90a3403b..fa95edb9 100644 --- a/driver/connect.c +++ b/driver/connect.c @@ -675,7 +675,7 @@ static BOOL config_dbc_logging(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs) WPFWP_LDESC "_" WPFWP_LDESC "_" "%d-%u", LWSTR(&attrs->server), LWSTR(&attrs->port), GetCurrentProcessId(), InterlockedIncrement(&filelog_cnt)); - if (cnt <= 0 || ident.cnt <= cnt) { + if (cnt <= 0 || ident.cnt <= (size_t)cnt) { ERRH(dbc, "failed to print log file identifier."); SET_HDIAG(dbc, SQL_STATE_HY000, "failed to print log file ID", 0); return FALSE; @@ -683,7 +683,7 @@ static BOOL config_dbc_logging(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs) ident.cnt = cnt; } /* replace reserved characters that could raise issues with the FS */ - for (cnt = 0; cnt < ident.cnt; cnt ++) { + for (cnt = 0; (size_t)cnt < ident.cnt; cnt ++) { if (ident.str[cnt] < 31) { ident.str[cnt] = L'_'; } else { diff --git a/driver/log.c b/driver/log.c index da293e78..6c74e5e1 100644 --- a/driver/log.c +++ b/driver/log.c @@ -54,10 +54,11 @@ BOOL log_init() cnt = GetEnvironmentVariable(MK_WPTR(ESODBC_LOG_DIR_ENV_VAR), dpath.str, (DWORD)dpath.cnt); + assert(0 <= cnt); if (! cnt) { /* 0 means error */ /* env var wasn't defined OR error occured (which we can't log). */ return GetLastError() == ERROR_ENVVAR_NOT_FOUND; - } else if (dpath.cnt <= cnt) { + } else if (dpath.cnt <= (size_t)cnt) { /* path buffer too small */ assert(0); return FALSE; @@ -157,7 +158,7 @@ BOOL filelog_print_path(wstr_st *dest, wstr_st *dir_path, wstr_st *ident) (int)ident->cnt, ident->str, MK_WPTR(ESODBC_LOG_FILE_SUFFIX)); - if (cnt <= 0 || dest->cnt <= cnt) { + if (cnt <= 0 || dest->cnt <= (size_t)cnt) { /* fpath buffer is too small */ return FALSE; } else { From d2dbe2fc0edb5e42449571abc1585103e6f29d26 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Wed, 21 Nov 2018 18:24:04 +0100 Subject: [PATCH 5/5] fix: simpler post size setting for libcurl - also remove a superfluous ';'. --- driver/catalogue.c | 2 +- driver/connect.c | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/driver/catalogue.c b/driver/catalogue.c index 0a961260..872283c0 100644 --- a/driver/catalogue.c +++ b/driver/catalogue.c @@ -199,7 +199,7 @@ SQLRETURN EsSQLTablesW( SQLSMALLINT NameLength4) { esodbc_stmt_st *stmt = STMH(StatementHandle); - SQLRETURN ret = SQL_ERROR;; + SQLRETURN ret = SQL_ERROR; enum { wbuf_cnt = sizeof(SQL_TABLES) + sizeof(SQL_TABLES_CAT) diff --git a/driver/connect.c b/driver/connect.c index fa95edb9..7a2e5b25 100644 --- a/driver/connect.c +++ b/driver/connect.c @@ -479,6 +479,7 @@ static BOOL dbc_curl_perform(esodbc_dbc_st *dbc, long *code, cstr_st *resp) static BOOL dbc_curl_prepare(esodbc_dbc_st *dbc, SQLULEN tout, const cstr_st *u8body) { + curl_off_t post_size = u8body->cnt; assert(dbc->curl); dbc->curl_err = CURLE_OK; @@ -492,21 +493,9 @@ static BOOL dbc_curl_prepare(esodbc_dbc_st *dbc, SQLULEN tout, } } - /* curl will always expect a 64bit value (curl_off_t), while size_t - * remains a 32bit type of 32bit platforms (where a long is needed). */ -#if defined(_WIN64) || defined (WIN64) /* len of the body */ dbc->curl_err = curl_easy_setopt(dbc->curl, CURLOPT_POSTFIELDSIZE_LARGE, - u8body->cnt); -#else /* 64b */ - if (LONG_MAX < u8body->cnt) { - ERRH(dbc, "libcurl: post fieldsize %zu too large; max: %ld.", - u8body->cnt, LONG_MAX); - goto err; - } - dbc->curl_err = curl_easy_setopt(dbc->curl, CURLOPT_POSTFIELDSIZE, - u8body->cnt); -#endif /* 64b */ + post_size); if (dbc->curl_err != CURLE_OK) { ERRH(dbc, "libcurl: failed to set post fieldsize: %zu.", u8body->cnt); goto err;