Skip to content

Commit

Permalink
add version check control parameter. leak fix (#90)
Browse files Browse the repository at this point in the history
A new parameter 'VersionChecking' is added, to control how strict the
version check is performed:
- strict: the default; will only connect to a server with same version;
- major: major versions must match;
- none: only avail in debug builds; no version check.

The commit also fixes a memory leak: the body of the GET (containing the
server version) is now freed.

(cherry picked from commit fa258fd)
  • Loading branch information
bpintea committed Jan 17, 2019
1 parent 83b8800 commit c51e33b
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 34 deletions.
96 changes: 69 additions & 27 deletions driver/connect.c
Expand Up @@ -1063,9 +1063,9 @@ SQLRETURN config_dbc(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs)
/*
* set the REST body format: JSON/CBOR
*/
if (EQ_CASE_WSTR(&attrs->packing, &MK_WSTR("JSON"))) {
if (EQ_CASE_WSTR(&attrs->packing, &MK_WSTR(ESODBC_DSN_PACK_JSON))) {
dbc->pack_json = TRUE;
} else if (EQ_CASE_WSTR(&attrs->packing, &MK_WSTR("CBOR"))) {
} else if (EQ_CASE_WSTR(&attrs->packing, &MK_WSTR(ESODBC_DSN_PACK_CBOR))) {
dbc->pack_json = FALSE;
} else {
ERRH(dbc, "unknown packing encoding '" LWPDL "'.",
Expand All @@ -1075,6 +1075,28 @@ SQLRETURN config_dbc(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs)
}
INFOH(dbc, "pack JSON: %s.", dbc->pack_json ? "true" : "false");

/*
* Version checking mode
*/
if (EQ_CASE_WSTR(&attrs->version_checking,
&MK_WSTR(ESODBC_DSN_VC_STRICT))
|| EQ_CASE_WSTR(&attrs->version_checking,
&MK_WSTR(ESODBC_DSN_VC_MAJOR))
# ifndef NDEBUG
|| EQ_CASE_WSTR(&attrs->version_checking,
&MK_WSTR(ESODBC_DSN_VC_NONE))
# endif /* NDEBUG */
) {
dbc->srv_ver.checking = (unsigned char)attrs->version_checking.str[0];
DBGH(dbc, "version checking mode: %c.", dbc->srv_ver.checking);
} else {
ERRH(dbc, "unknown version checking mode '" LWPDL "'.",
LWSTR(&attrs->version_checking));
SET_HDIAG(dbc, SQL_STATE_HY000, "invalid version checking mode "
"setting", 0);
goto err;
}

return SQL_SUCCESS;
err:
/* release allocated resources before the failure; not the diag, tho */
Expand Down Expand Up @@ -1146,6 +1168,11 @@ void cleanup_dbc(esodbc_dbc_st *dbc)
} else {
assert(dbc->no_types == 0);
}
if (dbc->srv_ver.string.cnt) { /* .str might be compromized by the union */
free(dbc->srv_ver.string.str);
dbc->srv_ver.string.str = NULL;
dbc->srv_ver.string.cnt = 0;
}

assert(dbc->abuff == NULL);
cleanup_curl(dbc);
Expand All @@ -1168,9 +1195,6 @@ static SQLRETURN check_server_version(esodbc_dbc_st *dbc)
const wchar_t *version_key[] = {L"number"};
wstr_st ver_no;
wstr_st own_ver = WSTR_INIT(STR(DRV_VERSION)); /*build-time define*/
# ifndef NDEBUG
SQLWCHAR *pos, *end;
# endif /* !NDEBUG */
static const wchar_t err_msg_fmt[] = L"Version mismatch between server ("
WPFWP_LDESC ") and driver (" WPFWP_LDESC "). Please use a driver whose"
" version matches that of your server.";
Expand Down Expand Up @@ -1223,34 +1247,51 @@ static SQLRETURN check_server_version(esodbc_dbc_st *dbc)

# ifndef NDEBUG
/* strip any qualifiers (=anything following a first `-`) in debug mode */
for (pos = ver_no.str, end = pos + ver_no.cnt; pos < end; pos ++) {
if (*pos == L'-') {
ver_no.cnt = pos - ver_no.str;
break;
}
}
for (pos = own_ver.str, end = pos + own_ver.cnt; pos < end; pos ++) {
if (*pos == L'-') {
own_ver.cnt = pos - own_ver.str;
break;
wtrim_at(&ver_no, L'-');
wtrim_at(&own_ver, L'-');
# endif /* !NDEBUG */

if (tolower(dbc->srv_ver.checking) == tolower(ESODBC_DSN_VC_MAJOR[0])) {
/* trim versions to the first dot, i.e. major version */
wtrim_at(&ver_no, L'.');
wtrim_at(&own_ver, L'.');
}

if (tolower(dbc->srv_ver.checking) != tolower(ESODBC_DSN_VC_NONE[0])) {
if (! EQ_WSTR(&ver_no, &own_ver)) {
ERRH(dbc, "version mismatch: server: " LWPDL ", "
"own: " LWPDL ".", LWSTR(&ver_no), LWSTR(&own_ver));
n = swprintf(wbuff, sizeof(wbuff)/sizeof(wbuff[0]),
err_msg_fmt, LWSTR(&ver_no), LWSTR(&own_ver));
ret = post_diagnostic(dbc, SQL_STATE_HY000, (n <= 0) ?
L"Version mismatch between server and driver" :
wbuff, 0);
} else {
INFOH(dbc, "server and driver versions aligned to: " LWPDL ".",
LWSTR(&own_ver));
ret = SQL_SUCCESS;
}
}
# ifndef NDEBUG
} else {
WARNH(dbc, "version checking disabled.");
# endif /* !NDEBUG */
}

if (! EQ_WSTR(&ver_no, &own_ver)) {
ERRH(dbc, "version mismatch: server: " LWPDL ", own: " LWPDL ".",
LWSTR(&ver_no), LWSTR(&own_ver));
n = swprintf(wbuff, sizeof(wbuff)/sizeof(wbuff[0]), err_msg_fmt,
LWSTR(&ver_no), LWSTR(&own_ver));
ret = post_diagnostic(dbc, SQL_STATE_HY000, (n <= 0) ?
L"Version mismatch between server and driver" :
wbuff, 0);
/* re-read the original version (before trimming) and dup it */
ver_no.str = (SQLWCHAR *)UJReadString(o_number, &ver_no.cnt);
/* version is returned to application, which requires a NTS => +1 for \0 */
if (! (dbc->srv_ver.string.str = malloc(ver_no.cnt * sizeof(SQLWCHAR) + /*\0*/1))) {
ERRNH(dbc, "OOM for %zd.", ver_no.cnt * sizeof(SQLWCHAR));
post_diagnostic(dbc, SQL_STATE_HY001, NULL, 0);
goto err;
} else {
INFOH(dbc, "server and driver versions aligned to: " LWPDL ".",
LWSTR(&own_ver));
ret = SQL_SUCCESS;
memcpy(dbc->srv_ver.string.str, ver_no.str,
ver_no.cnt * sizeof(SQLWCHAR));
dbc->srv_ver.string.cnt = ver_no.cnt;
dbc->srv_ver.string.str[ver_no.cnt] = 0;
}

free(resp.str);
assert(state);
UJFree(state);
return ret;
Expand All @@ -1259,6 +1300,7 @@ static SQLRETURN check_server_version(esodbc_dbc_st *dbc)
if (resp.cnt) {
ERRH(dbc, "failed to process server's answer: [%zu] `" LWPDL "`.",
resp.cnt, LCSTR(&resp));
free(resp.str);
}
if (state) {
UJFree(state);
Expand Down
4 changes: 3 additions & 1 deletion driver/defs.h
Expand Up @@ -164,7 +164,9 @@
#define ESODBC_DEF_TRACE_ENABLED "0"
/* default tracing level */
#define ESODBC_DEF_TRACE_LEVEL "WARN"
#define ESODBC_PWD_VAL_SUBST "<original substituted>"
#define ESODBC_PWD_VAL_SUBST "<redacted>"
/* default version checking mode: strict, major, none (dbg only) */
#define ESODBC_DEF_VERSION_CHECKING ESODBC_DSN_VC_STRICT

/*
*
Expand Down
11 changes: 11 additions & 0 deletions driver/dsn.c
Expand Up @@ -77,6 +77,7 @@ int assign_dsn_attr(esodbc_dsn_attrs_st *attrs,
{&MK_WSTR(ESODBC_DSN_PACKING), &attrs->packing},
{&MK_WSTR(ESODBC_DSN_MAX_FETCH_SIZE), &attrs->max_fetch_size},
{&MK_WSTR(ESODBC_DSN_MAX_BODY_SIZE_MB), &attrs->max_body_size},
{&MK_WSTR(ESODBC_DSN_VERSION_CHECKING), &attrs->version_checking},
{&MK_WSTR(ESODBC_DSN_TRACE_ENABLED), &attrs->trace_enabled},
{&MK_WSTR(ESODBC_DSN_TRACE_FILE), &attrs->trace_file},
{&MK_WSTR(ESODBC_DSN_TRACE_LEVEL), &attrs->trace_level},
Expand Down Expand Up @@ -404,6 +405,7 @@ long TEST_API write_00_list(esodbc_dsn_attrs_st *attrs,
{&MK_WSTR(ESODBC_DSN_PACKING), &attrs->packing},
{&MK_WSTR(ESODBC_DSN_MAX_FETCH_SIZE), &attrs->max_fetch_size},
{&MK_WSTR(ESODBC_DSN_MAX_BODY_SIZE_MB), &attrs->max_body_size},
{&MK_WSTR(ESODBC_DSN_VERSION_CHECKING), &attrs->version_checking},
{&MK_WSTR(ESODBC_DSN_TRACE_ENABLED), &attrs->trace_enabled},
{&MK_WSTR(ESODBC_DSN_TRACE_FILE), &attrs->trace_file},
{&MK_WSTR(ESODBC_DSN_TRACE_LEVEL), &attrs->trace_level},
Expand Down Expand Up @@ -673,6 +675,11 @@ BOOL write_system_dsn(esodbc_dsn_attrs_st *new_attrs,
&MK_WSTR(ESODBC_DSN_MAX_BODY_SIZE_MB), &new_attrs->max_body_size,
old_attrs ? &old_attrs->max_body_size : NULL
},
{
&MK_WSTR(ESODBC_DSN_VERSION_CHECKING),
&new_attrs->version_checking,
old_attrs ? &old_attrs->version_checking : NULL
},
{
&MK_WSTR(ESODBC_DSN_TRACE_ENABLED), &new_attrs->trace_enabled,
old_attrs ? &old_attrs->trace_enabled : NULL
Expand Down Expand Up @@ -757,6 +764,7 @@ long TEST_API write_connection_string(esodbc_dsn_attrs_st *attrs,
{&attrs->packing, &MK_WSTR(ESODBC_DSN_PACKING)},
{&attrs->max_fetch_size, &MK_WSTR(ESODBC_DSN_MAX_FETCH_SIZE)},
{&attrs->max_body_size, &MK_WSTR(ESODBC_DSN_MAX_BODY_SIZE_MB)},
{&attrs->version_checking, &MK_WSTR(ESODBC_DSN_VERSION_CHECKING)},
{&attrs->trace_enabled, &MK_WSTR(ESODBC_DSN_TRACE_ENABLED)},
{&attrs->trace_file, &MK_WSTR(ESODBC_DSN_TRACE_FILE)},
{&attrs->trace_level, &MK_WSTR(ESODBC_DSN_TRACE_LEVEL)},
Expand Down Expand Up @@ -835,6 +843,9 @@ void assign_dsn_defaults(esodbc_dsn_attrs_st *attrs)
res |= assign_dsn_attr(attrs, &MK_WSTR(ESODBC_DSN_MAX_BODY_SIZE_MB),
&MK_WSTR(ESODBC_DEF_MAX_BODY_SIZE_MB),
/*overwrite?*/FALSE);
res |= assign_dsn_attr(attrs, &MK_WSTR(ESODBC_DSN_VERSION_CHECKING),
&MK_WSTR(ESODBC_DEF_VERSION_CHECKING),
/*overwrite?*/FALSE);

/* default: no trace file */
res |= assign_dsn_attr(attrs,
Expand Down
12 changes: 11 additions & 1 deletion driver/dsn.h
Expand Up @@ -32,10 +32,19 @@
#define ESODBC_DSN_PACKING "Packing"
#define ESODBC_DSN_MAX_FETCH_SIZE "MaxFetchSize"
#define ESODBC_DSN_MAX_BODY_SIZE_MB "MaxBodySizeMB"
#define ESODBC_DSN_VERSION_CHECKING "VersionChecking"
#define ESODBC_DSN_TRACE_ENABLED "TraceEnabled"
#define ESODBC_DSN_TRACE_FILE "TraceFile"
#define ESODBC_DSN_TRACE_LEVEL "TraceLevel"

/* Packing values */
#define ESODBC_DSN_PACK_JSON "JSON"
#define ESODBC_DSN_PACK_CBOR "CBOR"
/* VersionChecking values */
#define ESODBC_DSN_VC_STRICT "strict"
#define ESODBC_DSN_VC_MAJOR "major"
#define ESODBC_DSN_VC_NONE "none"

/* stucture to collect all attributes in a connection string */
typedef struct {
wstr_st driver;
Expand All @@ -55,10 +64,11 @@ typedef struct {
wstr_st packing;
wstr_st max_fetch_size;
wstr_st max_body_size;
wstr_st version_checking;
wstr_st trace_enabled;
wstr_st trace_file;
wstr_st trace_level;
#define ESODBC_DSN_ATTRS_COUNT 20
#define ESODBC_DSN_ATTRS_COUNT 21
SQLWCHAR buff[ESODBC_DSN_ATTRS_COUNT * ESODBC_DSN_MAX_ATTR_LEN];
} esodbc_dsn_attrs_st;

Expand Down
4 changes: 4 additions & 0 deletions driver/handles.h
Expand Up @@ -130,6 +130,10 @@ typedef struct struct_dbc {

wstr_st dsn; /* data source name SQLGetInfo(SQL_DATA_SOURCE_NAME) */
wstr_st server; /* ~ name; requested with SQLGetInfo(SQL_SERVER_NAME) */
union {
wstr_st string; /* version: SQLGetInfo(SQL_DBMS_VER) */
unsigned char checking; /* first letter of DSN config option */
} srv_ver; /* server version */
cstr_st url; /* SQL URL (posts) */
cstr_st root_url; /* root URL (gets) */
enum {
Expand Down
5 changes: 3 additions & 2 deletions driver/info.c
Expand Up @@ -304,8 +304,9 @@ static SQLRETURN getinfo_dbms_product(
&MK_WSTR(ESODBC_ELASTICSEARCH_NAME), BufferLength,
StringLengthPtr);
case SQL_DBMS_VER:
DBGH(dbc, "requested: DBMS version (`%s`).", STR(DRV_VERSION));
return write_wstr(dbc, InfoValue, &MK_WSTR(STR(DRV_VERSION)),
DBGH(dbc, "requested: DBMS version (`" LWPDL "`).",
LWSTR(&dbc->srv_ver.string));
return write_wstr(dbc, InfoValue, &dbc->srv_ver.string,
BufferLength, StringLengthPtr);
}
*handled = FALSE;
Expand Down
18 changes: 16 additions & 2 deletions driver/util.c
Expand Up @@ -287,6 +287,18 @@ void trim_ws(cstr_st *cstr)
};
}

BOOL wtrim_at(wstr_st *wstr, SQLWCHAR wchar)
{
SQLWCHAR *pos, *end;

for (pos = wstr->str, end = pos + wstr->cnt; pos < end; pos ++) {
if (*pos == wchar) {
wstr->cnt = pos - wstr->str;
return TRUE;
}
}
return FALSE;
}

/*
* Converts a wchar_t string to a C string for ASCII characters.
Expand Down Expand Up @@ -398,7 +410,8 @@ static inline size_t json_escaped_len(const char *json, size_t len)
switch(uchar) {
case '"':
case '\\':
case '/':
/* '/' needs no quoting as per ECMA spec, even if listed on
* json.org; ES/SQL uses it unescaped in cursor values. */
case '\b':
case '\f':
case '\n':
Expand Down Expand Up @@ -449,7 +462,8 @@ size_t json_escape(const char *jin, size_t inlen, char *jout, size_t outlen)
} while (0);
case '"':
case '\\':
case '/':
/* '/' needs no quoting as per ECMA spec, even if listed on
* json.org; ES/SQL uses it unescaped in cursor values. */
if (outlen <= pos + 1) {
i = inlen; // break the for loop
continue;
Expand Down
4 changes: 3 additions & 1 deletion driver/util.h
Expand Up @@ -153,7 +153,9 @@ void trim_ws(cstr_st *str);
void wltrim_ws(wstr_st *wstr);
void wrtrim_ws(wstr_st *wstr);
#define wtrim_ws(_w) do { wltrim_ws(_w); wrtrim_ws(_w); } while (0)

/* Trim the w-string at first encounter of the give w-char.
* Returns TRUE if character has been encounter / trimming occured. */
BOOL wtrim_at(wstr_st *wstr, SQLWCHAR wchar);

BOOL wstr2bool(wstr_st *val);
/* Converts a [cw]str_st to a SQL(U)BIGINT.
Expand Down

0 comments on commit c51e33b

Please sign in to comment.