Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix checking if bind value is mysql number and if needs to be quoted #57

Merged
merged 2 commits into from May 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
209 changes: 98 additions & 111 deletions dbdimp.c
Expand Up @@ -21,7 +21,7 @@
return (value);\
}

static bool parse_number(char *string, STRLEN len, char **end);
static bool is_mysql_number(char *string, STRLEN len);

DBISTATE_DECLARE;

Expand Down Expand Up @@ -651,7 +651,6 @@ static char *parse_params(
bool comment_end = FALSE;
char *salloc, *statement_ptr;
char *statement_ptr_end, *ptr;
char *cp, *end;
int i;
STRLEN alen;
STRLEN slen = *slen_ptr;
Expand Down Expand Up @@ -679,24 +678,7 @@ static char *parse_params(
if (!ph->value)
alen+= 3; /* Erase '?', insert 'NULL' */
else
{
alen+= 2+ph->len+1;
/* this will most likely not happen since line 214 */
/* of mysql.xs hardcodes all types to SQL_VARCHAR */
if (!ph->type)
{
if (bind_type_guessing)
{
ph->type= SQL_INTEGER;
if (!parse_number(ph->value, ph->len, &end))
{
ph->type= SQL_VARCHAR;
}
}
else
ph->type= SQL_VARCHAR;
}
}
}

/* Allocate memory, why *2, well, because we have ptr and statement_ptr */
Expand Down Expand Up @@ -838,39 +820,33 @@ static char *parse_params(
}
else
{
bool is_num = FALSE;
bool quote_value, is_value_num;

is_value_num = is_mysql_number(ph->value, ph->len);

if (limit_flag && is_value_num)
/* After a LIMIT clause must be unquoted numeric value */
quote_value = FALSE;
else if (bind_type_guessing && !ph->type)
/* If SQL type was not specified and bind_type_guessing is enabled, then quote only if needed */
quote_value = !is_value_num;
else if (sql_type_is_numeric(ph->type))
/* If SQL type is numeric then quote only in case value is not numeric */
quote_value = !is_value_num;
else
/* Otherwise always quote */
quote_value = TRUE;

if (ph->value)
if (quote_value)
{
is_num = sql_type_is_numeric(ph->type);

/* (note this sets *end, which we use if is_num) */
if (!parse_number(ph->value, ph->len, &end) && is_num)
{
if (bind_type_guessing) {
/* .. not a number, so apparently we guessed wrong */
is_num = FALSE;
ph->type = SQL_VARCHAR;
}
}


/* we're at the end of the query, so any placeholders if */
/* after a LIMIT clause will be numbers and should not be quoted */
if (limit_flag)
is_num = TRUE;

if (!is_num)
{
*ptr++ = '\'';
ptr += mysql_real_escape_string(sock, ptr, ph->value, ph->len);
*ptr++ = '\'';
}
else
{
for (cp= ph->value; cp < end; cp++)
*ptr++= *cp;
}
*ptr++ = '\'';
ptr += mysql_real_escape_string(sock, ptr, ph->value, ph->len);
*ptr++ = '\'';
}
else
{
memcpy(ptr, ph->value, ph->len);
ptr += ph->len;
}
}
break;
Expand Down Expand Up @@ -1932,6 +1908,14 @@ static MYSQL *mariadb_dr_connect(
"imp_dbh->use_server_side_prepare: %d\n",
imp_dbh->use_server_side_prepare ? 1 : 0);

if (imp_dbh->bind_type_guessing && imp_dbh->use_server_side_prepare)
{
sock->net.last_errno = CR_CONNECTION_ERROR;
strcpy(sock->net.sqlstate, "HY000");
strcpy(sock->net.last_error, "Connection error: mariadb_bind_type_guessing and mariadb_server_prepare cannot be enabled together");
return NULL;
}

(void)hv_stores(processed, "mariadb_server_prepare_disable_fallback", &PL_sv_yes);
if ((svp = hv_fetchs(hv, "mariadb_server_prepare_disable_fallback", FALSE)) && *svp)
imp_dbh->disable_fallback_for_server_prepare = SvTRUE(*svp);
Expand Down Expand Up @@ -2729,13 +2713,27 @@ mariadb_db_STORE_attrib(
else if (memEQs(key, kl, "mariadb_auto_reconnect"))
imp_dbh->auto_reconnect = bool_value;
else if (memEQs(key, kl, "mariadb_server_prepare"))
{
if (bool_value && imp_dbh->bind_type_guessing)
{
mariadb_dr_do_error(dbh, CR_UNKNOWN_ERROR, "mariadb_bind_type_guessing and mariadb_server_prepare cannot be enabled together", "HY000");
return 0;
}
imp_dbh->use_server_side_prepare = bool_value;
}
else if (memEQs(key, kl, "mariadb_server_prepare_disable_fallback"))
imp_dbh->disable_fallback_for_server_prepare = bool_value;
else if (memEQs(key, kl, "mariadb_no_autocommit_cmd"))
imp_dbh->no_autocommit_cmd = bool_value;
else if (memEQs(key, kl, "mariadb_bind_type_guessing"))
{
if (bool_value && imp_dbh->use_server_side_prepare)
{
mariadb_dr_do_error(dbh, CR_UNKNOWN_ERROR, "mariadb_bind_type_guessing and mariadb_server_prepare cannot be enabled together", "HY000");
return 0;
}
imp_dbh->bind_type_guessing = bool_value;
}
else if (memEQs(key, kl, "mariadb_bind_comment_placeholders"))
imp_dbh->bind_comment_placeholders = bool_value;
#ifdef HAVE_FABRIC
Expand Down Expand Up @@ -5915,80 +5913,69 @@ int mariadb_db_async_ready(SV* h)
}
}

static bool parse_number(char *string, STRLEN len, char **end)
static bool is_mysql_number(char *string, STRLEN len)
{
int seen_neg = 0;
bool seen_dec = FALSE;
bool seen_e = FALSE;
bool seen_plus = FALSE;
char *cp;

if (len <= 0) {
len= strlen(string);
}
char *cp = string;
bool number_found = FALSE;

cp= string;
/* Skip leading MySQL utf8mb4 whitespaces */
while (*cp == ' ' || (*cp >= 9 && *cp <= 13))
cp++;

/* Skip leading whitespace */
while (*cp && isspace(*cp))
/* Optional '+' or '-' */
if (*cp == '-' || *cp == '+')
cp++;

for ( ; *cp; cp++)
/* Number before '.' */
while (*cp >= '0' && *cp <= '9')
{
if ('-' == *cp)
{
if (seen_neg >= 2)
{
/*
third '-'. number can contains two '-'.
because -1e-10 is valid number */
break;
}
seen_neg += 1;
}
else if ('.' == *cp)
{
if (seen_dec)
{
/* second '.' */
break;
}
seen_dec = TRUE;
}
else if ('e' == *cp)
{
if (seen_e)
{
/* second 'e' */
break;
}
seen_e = TRUE;
}
else if ('+' == *cp)
cp++;
number_found = TRUE;
}

/* Optional '.' */
if (*cp == '.')
{
cp++;

/* Number after '.' */
while (*cp >= '0' && *cp <= '9')
{
if (seen_plus)
{
/* second '+' */
break;
}
seen_plus = TRUE;
cp++;
number_found = TRUE;
}
else if (!isdigit(*cp))
}

/* No number found - error */
if (!number_found)
return FALSE;

/* Optional exponent */
if (*cp == 'e' || *cp == 'E')
{
cp++;
/* Search for number also in exponent */
number_found = FALSE;

/* Optional '+' or '-' in exponent */
if (*cp == '-' || *cp == '+')
cp++;

/* Exponent number */
while (*cp >= '0' && *cp <= '9')
{
/* Not sure why this was changed */
/* seen_digit= 1; */
break;
cp++;
number_found = TRUE;
}
}

*end= cp;
/* Skip trailing MySQL utf8mb4 whitespaces */
while (*cp == ' ' || (*cp >= 9 && *cp <= 13))
cp++;

/* length 0 -> not a number */
/* Need to revisit this */
/*if (len == 0 || string + len < cp || seen_digit == 0) {*/
if (len == 0 || string + len > cp) {
/* Check that we processed all characters */
if (string + len != cp)
return FALSE;
}

return TRUE;
return number_found;
}
10 changes: 9 additions & 1 deletion t/51bind_type_guessing.t
Expand Up @@ -11,7 +11,7 @@ use vars qw($test_dsn $test_user $test_password);

my $dbh = DbiTestConnect($test_dsn, $test_user, $test_password,
{ RaiseError => 1, PrintError => 1, AutoCommit => 0 });
plan tests => 26;
plan tests => 32;

ok $dbh->do("DROP TABLE IF EXISTS dbd_mysql_t51bind_type_guessing"), "drop table if exists dbd_mysql_t51bind_type_guessing";

Expand Down Expand Up @@ -69,11 +69,19 @@ ok $sth3= $dbh->prepare("insert into dbd_mysql_t51bind_type_guessing (str, num)
ok $rows= $sth3->execute(52.3, 44);
ok $rows= $sth3->execute('', ' 77');
ok $rows= $sth3->execute(undef, undef);
ok $rows= $sth3->execute('.', 100);
ok $rows= $sth3->execute('+', 101);
ok $rows= $sth3->execute('-', 102);
ok $rows= $sth3->execute('+10e+100', 103);
ok $rows= $sth3->execute('-12.E-100', 104);

ok $sth3= $dbh->prepare("select * from dbd_mysql_t51bind_type_guessing limit ?");
ok $rows= $sth3->execute(1);
ok $rows= $sth3->execute(' 1');
$sth3->finish();

my $ref = $dbh->selectall_arrayref(q(SELECT * FROM dbd_mysql_t51bind_type_guessing WHERE str IN ('', '.', '+', '-')));
is_deeply($ref, [ [ '', 77 ], [ '.', 100 ], [ '+', 101 ], [ '-', 102 ] ] );

ok $dbh->do("DROP TABLE dbd_mysql_t51bind_type_guessing");
ok $dbh->disconnect;