Skip Menu |
 

This queue is for tickets about the DBD-mysql CPAN distribution.

Report information
The Basics
Id: 18976
Status: resolved
Priority: 0/
Queue: DBD-mysql

People
Owner: CAPTTOFU [...] cpan.org
Requestors: MSCHILLI [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 3.0002_5
Fixed in: (no value)



Subject: Negative values for SQL_INTEGER rejected by bind_param()
Download (untitled) / with headers
text/plain 804b
Hi DBD::mysql maintainers, the latest DBD::mysql on CPAN, 3.0002_5, has the following problem: If you specify a negative integer on a column marked as SQL_INTEGER, as in $sth->bind_param(1,undef,SQL_INTEGER); $sth->execute(-1) or die $DBI::errstr; then the integer doesn't make it into the resulting SQL, resulting in a syntax error. It works fine for positive integers. Negative integers used to work in the 2.9008 series, and stopped working in 3.2.x. Reason is that dbdimp.c only accepts digits as numbers ignoring minus signs (-) and floating points (.). I've attached a patch that puts the number parser, employing some of the logic of a previous patch by dragonchild, into a subroutine. I've also added a test case. Would be great if you could apply it. Thanks! -- Mike Schilli
Subject: patch.txt
Download patch.txt
text/plain 4.1k
diff -Naur DBD-mysql-3.0002_5/dbdimp.c DBD-mysql-3.0002_5.patched/dbdimp.c --- DBD-mysql-3.0002_5/dbdimp.c Wed Feb 1 14:49:02 2006 +++ DBD-mysql-3.0002_5.patched/dbdimp.c Fri Apr 28 13:30:49 2006 @@ -418,9 +418,10 @@ bool bind_type_guessing) { - bool seen_neg, seen_dec; + int rc; char *salloc, *statement_ptr; char *statement_ptr_end, testchar, *ptr, *valbuf; + char *cp, *end; int alen, i, j; int slen= *slen_ptr; int limit_flag= 0; @@ -467,40 +468,9 @@ valbuf= SvPV(ph->value, vallen); ph->type= SQL_INTEGER; - /* patch from Dragonchild */ - seen_neg= 0; - seen_dec= 0; - for (j= 0; j < (int)vallen; ++j) + if(parse_number(valbuf, vallen, &end) != 0) { - testchar= *(valbuf+j); - if ('-' == testchar) - { - if (seen_neg) - { - ph->type= SQL_VARCHAR; - break; - } - else if (j) - { - ph->type= SQL_VARCHAR; - break; - } - seen_neg= 1; - } - else if ('.' == testchar) - { - if (seen_dec) - { - ph->type= SQL_VARCHAR; - break; - } - seen_dec= 1; - } - else if (!isdigit(testchar)) - { ph->type= SQL_VARCHAR; - break; - } } } else if (bind_type_guessing) @@ -616,13 +586,9 @@ } else { - while (vallen--) - { - c = *valbuf++; - if ((c < '0' || c > '9') && c != ' ') - break; - *ptr++= c; - } + parse_number(valbuf, vallen, &end); + for(cp = valbuf; cp < end; cp++) + *ptr++ = *cp; } } } @@ -4277,3 +4243,61 @@ return sv_2mortal(my_ulonglong2str(mysql_insert_id(&((imp_dbh_t*)imp_dbh)->mysql))); } #endif + +int parse_number( + char *string, + STRLEN len, + char **end +) { + int seen_neg; + int seen_dec; + char *cp; + + seen_neg= 0; + seen_dec= 0; + + if(len <= 0) { + len = strlen(string); + } + + cp = string; + + for(cp = string; *cp; cp++) + { + if ('-' == *cp) + { + if (seen_neg) + { + /* second '-' */ + break; + } + else if (cp > string) + { + /* '-' after digit(s) */ + break; + } + seen_neg= 1; + } + else if ('.' == *cp) + { + if (seen_dec) + { + /* second '.' */ + break; + } + seen_dec= 1; + } + else if (!isdigit(*cp)) + { + break; + } + } + + *end = cp; + + if(cp - string < len) { + return -1; + } + + return 0; +} diff -Naur DBD-mysql-3.0002_5/t/40bindparam.t DBD-mysql-3.0002_5.patched/t/40bindparam.t --- DBD-mysql-3.0002_5/t/40bindparam.t Wed Feb 1 10:52:23 2006 +++ DBD-mysql-3.0002_5.patched/t/40bindparam.t Fri Apr 28 13:32:50 2006 @@ -130,6 +130,14 @@ or DbiError($dbh->err, $dbh->errstr); } + # Testing bugfix for negative integers + Test($state or $cursor->bind_param(1, undef, SQL_INTEGER())) + or DbiError($dbh->err, $dbh->errstr); + Test($state or $cursor->bind_param(2, undef)) + or DbiError($dbh->err, $dbh->errstr); + Test($state or $cursor->execute(-1, "abc")) + or DbiError($dbh->err, $dbh->errstr); + Test($state or undef $cursor || 1); # @@ -144,6 +152,11 @@ Test($state or $cursor->bind_columns(undef, \$id, \$name)) or DbiError($dbh->err, $dbh->errstr); + + Test($state or ($ref = $cursor->fetch) && $id == -1 && + $name eq 'abc') + or printf("Query returned id = %s, name = %s, ref = %s, %d\n", + $id, $name, $ref, scalar(@$ref)); Test($state or ($ref = $cursor->fetch) && $id == 1 && $name eq 'Alligator Descartes')
Download (untitled) / with headers
text/plain 196b
On Fri Apr 28 17:26:42 2006, MSCHILLI wrote: Show quoted text
> I've also added a test case.
Just found that my previous patch didn't allow for leading whitespace. Fixed, new patched attached. -- Mike Schilli
Download patch.txt
text/plain 4.5k
--- dbdimp.c Wed Feb 1 14:49:02 2006 +++ /dbdimp.c Fri Apr 28 15:24:32 2006 @@ -25,8 +25,6 @@ typedef short WORD; #endif - - DBISTATE_DECLARE; typedef struct sql_type_info_s @@ -418,9 +416,10 @@ bool bind_type_guessing) { - bool seen_neg, seen_dec; + int rc; char *salloc, *statement_ptr; char *statement_ptr_end, testchar, *ptr, *valbuf; + char *cp, *end; int alen, i, j; int slen= *slen_ptr; int limit_flag= 0; @@ -467,40 +466,9 @@ valbuf= SvPV(ph->value, vallen); ph->type= SQL_INTEGER; - /* patch from Dragonchild */ - seen_neg= 0; - seen_dec= 0; - for (j= 0; j < (int)vallen; ++j) + if(parse_number(valbuf, vallen, &end) != 0) { - testchar= *(valbuf+j); - if ('-' == testchar) - { - if (seen_neg) - { - ph->type= SQL_VARCHAR; - break; - } - else if (j) - { - ph->type= SQL_VARCHAR; - break; - } - seen_neg= 1; - } - else if ('.' == testchar) - { - if (seen_dec) - { - ph->type= SQL_VARCHAR; - break; - } - seen_dec= 1; - } - else if (!isdigit(testchar)) - { ph->type= SQL_VARCHAR; - break; - } } } else if (bind_type_guessing) @@ -616,13 +584,9 @@ } else { - while (vallen--) - { - c = *valbuf++; - if ((c < '0' || c > '9') && c != ' ') - break; - *ptr++= c; - } + parse_number(valbuf, vallen, &end); + for(cp = valbuf; cp < end; cp++) + *ptr++ = *cp; } } } @@ -2520,6 +2484,7 @@ { D_imp_dbh_from_sth; + int num_fields; int use_mysql_use_result=imp_sth->use_mysql_use_result; int next_result_return_code, i; MYSQL_RES** result= &imp_sth->result; @@ -2620,7 +2585,7 @@ /** Store the result in the current statement handle */ DBIc_ACTIVE_on(imp_sth); - int num_fields=mysql_num_fields(imp_sth->result); + num_fields=mysql_num_fields(imp_sth->result); DBIc_NUM_FIELDS(imp_sth) = num_fields; @@ -4277,3 +4242,65 @@ return sv_2mortal(my_ulonglong2str(mysql_insert_id(&((imp_dbh_t*)imp_dbh)->mysql))); } #endif + +int parse_number( + char *string, + STRLEN len, + char **end +) { + int seen_neg; + int seen_dec; + char *cp; + + seen_neg= 0; + seen_dec= 0; + + if(len <= 0) { + len = strlen(string); + } + + cp = string; + + /* Skip leading whitespace */ + while(*cp && isspace(*cp)) + cp++; + + for(; *cp; cp++) + { + if ('-' == *cp) + { + if (seen_neg) + { + /* second '-' */ + break; + } + else if (cp > string) + { + /* '-' after digit(s) */ + break; + } + seen_neg= 1; + } + else if ('.' == *cp) + { + if (seen_dec) + { + /* second '.' */ + break; + } + seen_dec= 1; + } + else if (!isdigit(*cp)) + { + break; + } + } + + *end = cp; + + if(cp - string < len) { + return -1; + } + + return 0; +} --- t/40bindparam.t Wed Feb 1 10:52:23 2006 +++ t/40bindparam.t Fri Apr 28 14:30:05 2006 @@ -130,6 +130,14 @@ or DbiError($dbh->err, $dbh->errstr); } + # Testing bugfix for negative integers + Test($state or $cursor->bind_param(1, undef, SQL_INTEGER())) + or DbiError($dbh->err, $dbh->errstr); + Test($state or $cursor->bind_param(2, undef)) + or DbiError($dbh->err, $dbh->errstr); + Test($state or $cursor->execute(-1, "abc")) + or DbiError($dbh->err, $dbh->errstr); + Test($state or undef $cursor || 1); # @@ -144,6 +152,11 @@ Test($state or $cursor->bind_columns(undef, \$id, \$name)) or DbiError($dbh->err, $dbh->errstr); + + Test($state or ($ref = $cursor->fetch) && $id == -1 && + $name eq 'abc') + or printf("Query returned id = %s, name = %s, ref = %s, %d\n", + $id, $name, $ref, scalar(@$ref)); Test($state or ($ref = $cursor->fetch) && $id == 1 && $name eq 'Alligator Descartes')
From: Brad Lanam <bll [...] gentoo.com>
I also have this bug in DBD-mysql-3.0006.
Sorry I missed this great patch - I will put this in my queue for 3.0008 next week.
Download (untitled) / with headers
text/plain 136b
This fix has been applied to the DBD::mysql repository, and will be included in the next release (probably 4.01). Thanks for the patch!
CC: MSCHILLI [...] cpan.org
Subject: Re: [rt.cpan.org #18976] Negative values for SQL_INTEGER rejected by bind_param()
Date: Thu, 4 Jan 2007 17:06:56 -0800 (PST)
To: Jim Winstead via RT <bug-DBD-mysql [...] rt.cpan.org>
From: Mike Schilli <m [...] perlmeister.com>
Download (untitled) / with headers
text/plain 246b
On Thu, 28 Dec 2006, Jim Winstead via RT wrote: Show quoted text
> This fix has been applied to the DBD::mysql repository, and will be > included in the next release (probably 4.01). Thanks for the patch!
Sweet, thanks! -- Mike Mike Schilli m@perlmeister.com
This patch has been in the code since 3.0004


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.