Skip Menu |

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

Report information
The Basics
Id: 79576
Status: resolved
Priority: 0/
Queue: DBD-SQLite

People
Owner: Nobody in particular
Requestors: VOVKASM [...] cpan.org
Cc: ribasushi [...] leporine.io
AdminCc:

Bug Information
Severity: Important
Broken in: 1.37
Fixed in: (no value)



Subject: (patch) bind_param don't work with PADTMP scalars
This snippet of code fails (extracted from DBIx::Class code, so may occur in many-many projects): my $sth = $dbh->prepare("UPDATE test SET a = ?, b = ? WHERE id = ?"); my @values = ("hello","world",5); for (my $i=0;$i<3;$i++) { # $sth->bind_param($i+1, $values[$i]); # work as expected $sth->bind_param($i+1, "$values[$i]"); # this not work!! } $sth->execute(); # in second case this statement sended to sqlite engine: # UPDATE test SET a = 5, b = 5 WHERE id = 5 It because modern perl reused SV in second case. Patch dbd-sqlite-bind-param-should-work-with-PADTMPs.patch should fix this by always copy value in sqlite_bind_ph before saving for later use. Summary of my perl5 (revision 5 version 16 subversion 1) configuration: Platform: osname=linux, osvers=3.2.0-31-generic, archname=x86_64-linux uname='linux vovkasm-wrk 3.2.0-31-generic #50-ubuntu smp fri sep 7 16:16:45 utc 2012 x86_64 x86_64 x86_64 gnulinux ' config_args='-de -Dprefix=/home/vovkasm/perl5/perlbrew/perls/perl- 5.16.1-nothreads -Uusethreads' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector - I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -fstack-protector - I/usr/local/include' ccversion='', gccversion='4.6.3', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lnsl -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.15' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack- protector' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP PERL_PRESERVE_IVUV USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF
Subject: dbd-sqlite-bind-param-should-work-with-PADTMPs.patch
Index: t/19_bindparam.t =================================================================== --- t/19_bindparam.t (revision 15525) +++ t/19_bindparam.t (working copy) @@ -7,7 +7,7 @@ } use t::lib::Test; -use Test::More tests => 33; +use Test::More tests => 39; use Test::NoWarnings; use DBI ':sql_types'; @@ -48,6 +48,13 @@ ok( $sth->bind_param(1, 5, SQL_INTEGER), 'bind 3' ); ok( $sth->bind_param(2, undef), 'bind 4' ); ok( $sth->execute, '->execute' ); + + # Works with PADTMPs? + my @values = (6, "Larry"); + for (my $i=0; $i<2; $i++) { + ok( $sth->bind_param($i+1, "$values[$i]"), 'bind '.($i+5) ); + } + ok( $sth->execute, '->execute' ); } # Reconnect @@ -75,4 +82,7 @@ ok( $sth->fetch, '->fetch' ); is( $id, 5, 'id = 5' ); is( $name, undef, 'name = undef' ); + ok( $sth->fetch, '->fetch' ); + is( $id, 6, 'id = 6' ); + is( $name, 'Larry', 'name = Larry' ); } Index: dbdimp.c =================================================================== --- dbdimp.c (revision 15525) +++ dbdimp.c (working copy) @@ -1197,7 +1197,7 @@ pos = 2 * (SvIV(param) - 1); } sqlite_trace(sth, imp_sth, 3, form("bind into 0x%p: %"IVdf" => %s (%"IVdf") pos %d", imp_sth->params, SvIV(param), SvPV_nolen_undef_ok(value), sql_type, pos)); - av_store(imp_sth->params, pos, SvREFCNT_inc(value)); + av_store(imp_sth->params, pos, newSVsv(value)); if (sql_type) { av_store(imp_sth->params, pos+1, newSViv(sql_type)); }
On Tue Sep 11 15:01:38 2012, VOVKASM wrote: Show quoted text
> This snippet of code fails (extracted from DBIx::Class code, so may > occur in many-many projects): > > my $sth = $dbh->prepare("UPDATE test SET a = ?, b = ? WHERE id = ?"); > my @values = ("hello","world",5); > for (my $i=0;$i<3;$i++) { > # $sth->bind_param($i+1, $values[$i]); # work as expected > $sth->bind_param($i+1, "$values[$i]"); # this not work!! > } > $sth->execute(); > # in second case this statement sended to sqlite engine: > # UPDATE test SET a = 5, b = 5 WHERE id = 5 > > It because modern perl reused SV in second case. > > Patch dbd-sqlite-bind-param-should-work-with-PADTMPs.patch should fix > this by always copy value in sqlite_bind_ph before saving for later use.
To be clear, this problem is not specific to PADTMPs. It will happen with any caller’s value that is subsequently modified.
Show quoted text
> To be clear, this problem is not specific to PADTMPs. It will happen > with any caller’s value > that is subsequently modified.
Yes, my bad. This also fail of course: my @vars = (6,"Larry"); my $var; for (my $i=0; $i<2; $i++) { $var = $vars[$i]; $sth->bind_param($i+1,$var); } $sth->execute;
Applied in the trunk. Thanks. On Wed Sep 12 04:01:38 2012, VOVKASM wrote: Show quoted text
> This snippet of code fails (extracted from DBIx::Class code, so may > occur in many-many projects): > > my $sth = $dbh->prepare("UPDATE test SET a = ?, b = ? WHERE id = ?"); > my @values = ("hello","world",5); > for (my $i=0;$i<3;$i++) { > # $sth->bind_param($i+1, $values[$i]); # work as expected > $sth->bind_param($i+1, "$values[$i]"); # this not work!! > } > $sth->execute(); > # in second case this statement sended to sqlite engine: > # UPDATE test SET a = 5, b = 5 WHERE id = 5 > > It because modern perl reused SV in second case. > > Patch dbd-sqlite-bind-param-should-work-with-PADTMPs.patch should fix > this by always copy value in sqlite_bind_ph before saving for later
use. Show quoted text
> > > > Summary of my perl5 (revision 5 version 16 subversion 1)
configuration: Show quoted text
> > Platform: > osname=linux, osvers=3.2.0-31-generic, archname=x86_64-linux > uname='linux vovkasm-wrk 3.2.0-31-generic #50-ubuntu smp fri sep
7 Show quoted text
> 16:16:45 utc 2012 x86_64 x86_64 x86_64 gnulinux ' > config_args='-de -Dprefix=/home/vovkasm/perl5/perlbrew/perls/perl- > 5.16.1-nothreads -Uusethreads' > hint=recommended, useposix=true, d_sigaction=define > useithreads=undef, usemultiplicity=undef > useperlio=define, d_sfio=undef, uselargefiles=define,
usesocks=undef Show quoted text
> use64bitint=define, use64bitall=define, uselongdouble=undef > usemymalloc=n, bincompat5005=undef > Compiler: > cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector - > I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', > optimize='-O2', > cppflags='-fno-strict-aliasing -pipe -fstack-protector - > I/usr/local/include' > ccversion='', gccversion='4.6.3', gccosandvers='' > intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 > d_longlong=define, longlongsize=8, d_longdbl=define,
longdblsize=16 Show quoted text
> ivtype='long', ivsize=8, nvtype='double', nvsize=8,
Off_t='off_t', Show quoted text
> lseeksize=8 > alignbytes=8, prototype=define > Linker and Libraries: > ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' > libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib > /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib > libs=-lnsl -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat > perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc > libc=, so=so, useshrplib=false, libperl=libperl.a > gnulibc_version='2.15' > Dynamic Linking: > dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' > cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -
fstack- Show quoted text
> protector' > > > Characteristics of this binary (from libperl): > Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV > PERL_MALLOC_WRAP PERL_PRESERVE_IVUV > USE_64_BIT_ALL > USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE > USE_LOCALE_COLLATE USE_LOCALE_CTYPE > USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF
DBD::SQLite 1.38_01 with the patch is released. Thanks. On Wed Sep 12 05:33:20 2012, VOVKASM wrote: Show quoted text
> > To be clear, this problem is not specific to PADTMPs. It will
happen Show quoted text
> > with any caller’s value > > that is subsequently modified.
> > Yes, my bad. This also fail of course: > my @vars = (6,"Larry"); > my $var; > for (my $i=0; $i<2; $i++) { > $var = $vars[$i]; > $sth->bind_param($i+1,$var); > } > $sth->execute; >
On Mon Sep 24 15:00:19 2012, ISHIGAKI wrote: Show quoted text
> DBD::SQLite 1.38_01 with the patch is released. Thanks.
Anything preventing this from getting shipped? This is a rather nasty and hard to detect bug. DBIC just shipped with a workaround for it, but there are many other users...
You mean, shipped as a non-developer's release? I don't think there is a particular blocker, and I have just sent the second reminder to adamk to release the next (developer's version first, then 1.39). On Thu Apr 04 20:10:48 2013, RIBASUSHI wrote: Show quoted text
> On Mon Sep 24 15:00:19 2012, ISHIGAKI wrote:
> > DBD::SQLite 1.38_01 with the patch is released. Thanks.
> > Anything preventing this from getting shipped? This is a rather nasty > and hard to detect bug. DBIC just shipped with a workaround for it, > but there are many other users...
On Thu Apr 04 13:18:49 2013, ISHIGAKI wrote: Show quoted text
> You mean, shipped as a non-developer's release? I don't think there is > a particular blocker, and I have just sent the second reminder to > adamk to release the next (developer's version first, then 1.39).
Blocker for me also. 1.83_03 is fine. It also fixes RT#84906 global buffer overflows in unix and unix-code -- Reini Urban
Closed as DBD::SQLite 1.39 was released. Thanks everyone.