Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fix conflicts for merge #3

Closed
wants to merge 7 commits into from

1 participant

@ginesr
Collaborator

No description provided.

wu-lee added some commits
wu-lee Fix bug RT#66815 - check statement handles are valid before using them
$dbh->prepare does not guarantee to return a valid statement handle.
It may return a false value.  So check the returned value, and abort
if it is false, otherwise the error caused ->prepare to fail will get
overwritten by a less helpful one which says "Can't call method
'execute' on an undefined value" error.
6d8932e
wu-lee A test showing DBD::Mock->begin_work, commit and rollback don't detec…
…t ->prepare failure

This is reported in RT ticket #66815

Basically any call to ->begin_work, ->commit, >rollback made when
their internal call to ->prepare returns an invalid statement handle
will not notice and proceed to use it anyway.

In particular, this happens when a DBD::Mock::Session is in effect,
and the session state list has been exhausted.  When this happens, the
method in question fails with an unhelpful error "Can't use an
undefined value as an ARRAY reference at ../lib/DBD/Mock.pm line
635", masking the real reason.
2c12ab9
wu-lee Merge branch 'rt66815'
* rt66815:
  Fix bug RT#66815 - check statement handles are valid before using them
  A test showing DBD::Mock->begin_work, commit and rollback don't detect ->prepare failure
4dcccc7
wu-lee Add DBD::Mock::Session->current_state method
This just makes the code simpler to follow. Particularly the portion
in validate_bound_params where results are loaded into the $tracker
object.
23af0c7
wu-lee A test showing DBD::Mock::Session->verify_bound_params doesn't check …
…for session exhaustion

Basically it succeeds despite there being no states left to verify,
and on return the ->execute method fails with an unhelpful error
"Can't use an undefined value # as an ARRAY reference at
../lib/DBD/Mock.pm line 635."
3a1d41b
wu-lee Fix DBD::Mock::Session->verify_bound_params to check for state exhaus…
…tion
179e83a
wu-lee Merge branch 'fix-verify-bound-params'
* fix-verify-bound-params:
  Fix DBD::Mock::Session->verify_bound_params to check for state exhaustion
  A test showing DBD::Mock::Session->verify_bound_params doesn't check for session exhaustion
  Add DBD::Mock::Session->current_state method
09b2b02
@ginesr ginesr closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 30, 2011
  1. Fix bug RT#66815 - check statement handles are valid before using them

    wu-lee authored
    $dbh->prepare does not guarantee to return a valid statement handle.
    It may return a false value.  So check the returned value, and abort
    if it is false, otherwise the error caused ->prepare to fail will get
    overwritten by a less helpful one which says "Can't call method
    'execute' on an undefined value" error.
  2. A test showing DBD::Mock->begin_work, commit and rollback don't detec…

    wu-lee authored
    …t ->prepare failure
    
    This is reported in RT ticket #66815
    
    Basically any call to ->begin_work, ->commit, >rollback made when
    their internal call to ->prepare returns an invalid statement handle
    will not notice and proceed to use it anyway.
    
    In particular, this happens when a DBD::Mock::Session is in effect,
    and the session state list has been exhausted.  When this happens, the
    method in question fails with an unhelpful error "Can't use an
    undefined value as an ARRAY reference at ../lib/DBD/Mock.pm line
    635", masking the real reason.
  3. Merge branch 'rt66815'

    wu-lee authored
    * rt66815:
      Fix bug RT#66815 - check statement handles are valid before using them
      A test showing DBD::Mock->begin_work, commit and rollback don't detect ->prepare failure
  4. Add DBD::Mock::Session->current_state method

    wu-lee authored
    This just makes the code simpler to follow. Particularly the portion
    in validate_bound_params where results are loaded into the $tracker
    object.
  5. A test showing DBD::Mock::Session->verify_bound_params doesn't check …

    wu-lee authored
    …for session exhaustion
    
    Basically it succeeds despite there being no states left to verify,
    and on return the ->execute method fails with an unhelpful error
    "Can't use an undefined value # as an ARRAY reference at
    ../lib/DBD/Mock.pm line 635."
  6. Merge branch 'fix-verify-bound-params'

    wu-lee authored
    * fix-verify-bound-params:
      Fix DBD::Mock::Session->verify_bound_params to check for state exhaustion
      A test showing DBD::Mock::Session->verify_bound_params doesn't check for session exhaustion
      Add DBD::Mock::Session->current_state method
This page is out of date. Refresh to see the latest.
Showing with 195 additions and 7 deletions.
  1. +23 −7 lib/DBD/Mock.pm
  2. +127 −0 t/bug_0002.t
  3. +45 −0 t/bug_0003.t
View
30 lib/DBD/Mock.pm
@@ -322,7 +322,8 @@ sub prepare {
if ($dbh->FETCH('AutoCommit')) {
$dbh->STORE('AutoCommit', 0);
$begin_work_commit = 1;
- my $sth = $dbh->prepare( 'BEGIN WORK' );
+ my $sth = $dbh->prepare( 'BEGIN WORK' )
+ or return;
my $rc = $sth->execute();
$sth->finish();
return $rc;
@@ -338,7 +339,8 @@ sub prepare {
return $dbh->set_err(1, "commit ineffective with AutoCommit" );
}
- my $sth = $dbh->prepare( 'COMMIT' );
+ my $sth = $dbh->prepare( 'COMMIT' )
+ or return;
my $rc = $sth->execute();
$sth->finish();
@@ -356,7 +358,8 @@ sub prepare {
return $dbh->set_err(1, "rollback ineffective with AutoCommit" );
}
- my $sth = $dbh->prepare( 'ROLLBACK' );
+ my $sth = $dbh->prepare( 'ROLLBACK' )
+ or return;
my $rc = $sth->execute();
$sth->finish();
@@ -619,9 +622,12 @@ sub execute {
if (my $session = $dbh->{mock_session}) {
eval {
+ my $state = $session->current_state;
$session->verify_bound_params($dbh, $tracker->bound_params());
- my $idx = $session->{state_index} - 1;
- my @results = @{$session->{states}->[$idx]->{results}};
+
+ # Load a copy of the results to return (minus the field
+ # names) into the tracker
+ my @results = @{$state->{results}};
shift @results;
$tracker->{return_data} = \@results;
};
@@ -1190,6 +1196,12 @@ sub name { (shift)->{name} }
sub reset { (shift)->{state_index} = 0 }
sub num_states { scalar( @{ (shift)->{states} } ) }
+sub current_state {
+ my $self = shift;
+ my $idx = $self->{state_index};
+ return $self->{states}[$idx];
+}
+
sub has_states_left {
my $self = shift;
return $self->{state_index} < scalar(@{$self->{states}});
@@ -1201,7 +1213,7 @@ sub verify_statement {
($self->has_states_left)
|| die "Session states exhausted, only '" . scalar(@{$self->{states}}) . "' in DBD::Mock::Session (" . $self->{name} . ")";
- my $current_state = $self->{states}->[$self->{state_index}];
+ my $current_state = $self->current_state;
# make sure our state is good
(exists ${$current_state}{statement} && exists ${$current_state}{results})
|| die "Bad state '" . $self->{state_index} . "' in DBD::Mock::Session (" . $self->{name} . ")";
@@ -1233,7 +1245,11 @@ sub verify_statement {
sub verify_bound_params {
my ($self, $dbh, $params) = @_;
- my $current_state = $self->{states}->[$self->{state_index}];
+
+ ($self->has_states_left)
+ || die "Session states exhausted, only '" . scalar(@{$self->{states}}) . "' in DBD::Mock::Session (" . $self->{name} . ")";
+
+ my $current_state = $self->current_state;
if (exists ${$current_state}{bound_params}) {
my $expected = $current_state->{bound_params};
(scalar(@{$expected}) == scalar(@{$params}))
View
127 t/bug_0002.t
@@ -0,0 +1,127 @@
+#!/usr/bin/perl
+use Test::More tests => 15;
+use strict;
+use warnings;
+use Test::Exception;
+use DBI;
+use DBD::Mock;
+
+# This test is designed to expose the bug found in the DBD::Mock
+# methods begin_work, commit and rollback (RT #66815), where a failing
+# ->prepare invocation (returning nothing) is not detected and the
+# undefined value resulting is used anyway. In this test, as in the
+# example found in the wild, the failure is triggered by exhaustion of
+# the session states.
+#
+# This is a list of sessions designed to engineer the right condition
+# to trigger the bug. They all start with a dummy statement (so that
+# there are at least two states) then the final statement is removed
+# before it is passed to DBD::Mock::Session->new (which requires at
+# least one state). The final statements are 'BEGIN WORK', 'COMMIT'
+# and 'ROLLBACK', respectively.
+#
+# Hence, when the test tries to invoke the final state, the session
+# will have run out and DBD::Mock->verify_statement will cause the
+# prepare method to fail.
+
+my @cases = (
+ 'begin_work' => [
+ {
+ statement => 'SELECT something FROM somewhere',
+ results => [],
+ },
+ {
+ statement => 'BEGIN WORK',
+ results => [],
+ },
+ ],
+
+ 'commit' => [
+ {
+ statement => 'SELECT something FROM somewhere',
+ results => [],
+ },
+ {
+ statement => 'BEGIN WORK',
+ results => [],
+ },
+ {
+ statement => 'INSERT INTO foo (bar) VALUES (?);',
+ results => [],
+ bound_params => [1],
+ },
+ {
+ statement => 'COMMIT',
+ results => [],
+ },
+ ],
+
+ 'rollback' => [
+ {
+ statement => 'SELECT something FROM somewhere',
+ results => [],
+ },
+ {
+ statement => 'BEGIN WORK',
+ results => [],
+ },
+ {
+ statement => 'INSERT INTO foo (bar) VALUES (?);',
+ results => [],
+ bound_params => [1],
+ },
+ {
+ statement => 'ROLLBACK',
+ results => [],
+ },
+ ],
+);
+
+while(@cases) {
+ my ($name, $states) = splice @cases, 0, 2;
+ my $case_name = "case $name";
+
+ my $dbh = DBI->connect('dbi:Mock:', '', '',
+ { PrintError => 0,
+ RaiseError => 1 });
+
+ # Add all but the last state of the expected session
+ my $missing_state = pop @$states;
+ my $num_states = @$states;
+ $dbh->{mock_session} = DBD::Mock::Session->new($name => @$states);
+
+ # Execute the initial dummy statement.
+ my $state = $states->[0];
+ my $sth = $dbh->prepare($state->{statement});
+ ok $sth,
+ "$case_name: prepare statement";
+
+ ok $sth->execute(),
+ "$case_name: execute statement";
+
+ # Now try and do the next steps in @session, but using the
+ # appropriate transaction methods directly. This should fail when
+ # the session is exhausted with a useful message. (The original
+ # bug meant that the message got clobbered by "Can't call method
+ # 'execute' on an undefined value".)
+ throws_ok {
+ # This statement is always the same.
+ ok $dbh->begin_work,
+ "$case_name: start transaction";
+
+ my $state = $states->[2];
+ my $sth = $dbh->prepare($state->{statement});
+ ok $sth,
+ "$case_name: prepare statement";
+
+ ok $sth->execute(@{$state->{bound_params}}),
+ "$case_name: execute statement";
+
+ # get the final operation from the session
+ my $operation = lc $missing_state->{statement};
+
+ ok $dbh->$operation,
+ "$case_name: $operation transaction";
+ } qr/\QSession states exhausted, only '$num_states' in DBD::Mock::Session\E/;
+
+}
View
45 t/bug_0003.t
@@ -0,0 +1,45 @@
+#!/usr/bin/perl
+use Test::More tests => 3;
+use strict;
+use warnings;
+use Test::Exception;
+use DBI;
+use DBD::Mock;
+
+# This tests that spurious extra ->execute invocations fail with a
+# useful message. This is because there was a bug in which
+# DBD::Mock->verify_bound_params didn't check that the session had run
+# out, and on return out-of-bounds element of the state array is
+# accessed, causing an unhelpful error "Can't use an undefined value
+# as an ARRAY reference at ../lib/DBD/Mock.pm line 635."
+
+my @session = (
+ {
+ 'statement' => 'INSERT INTO foo (bar) values (?);',
+ 'results' => [],
+ 'bound_params' => [1]
+ },
+);
+
+my $dbh = DBI->connect('dbi:Mock:', '', '', { PrintError => 0, RaiseError => 1});
+
+# Add just part of the expected session, such that the next step would be a 'BEGIN WORK'
+$dbh->{mock_session} = DBD::Mock::Session->new(@session);
+
+# now execute the steps in the session
+my $step = $session[0];
+
+my $sth = $dbh->prepare($step->{statement});
+ok $sth,
+ "prepare statement";
+
+my $params = $step->{bound_params} || [];
+ok $sth->execute(@$params),
+ "execute statement";
+
+# Session expects that to be all. So let's surprise it with another
+# ->execute. It should fail appropriately.
+throws_ok {
+ ok $sth->execute(@$params),
+} qr/\QSession states exhausted, only '1' in DBD::Mock::Session\E/,
+ "fails on executing one too many times";
Something went wrong with that request. Please try again.