php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #73530 Unsetting result set may reset other result set
Submitted: 2016-11-15 18:53 UTC Modified: 2017-01-13 10:39 UTC
Votes:3
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: cmb@php.net Assigned: cmb
Status: Re-Opened Package: SQLite related
PHP Version: master-Git-2016-11-15 (Git) OS: *
Private report: No CVE-ID:
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please — but make sure to vote on the bug!
Your email address:
MUST BE VALID
Solve the problem:
14 + 42 = ?
Subscribe to this entry?

 
 [2016-11-15 18:53 UTC] cmb@php.net
Description:
------------
Due to the implementation, interference of multiple result set
objects retrieved by executing the same statement object are to be
expected[1]. However, unsetting a result set while iterating
over another shouldn't reset the other result set.

[1] <http://svn.php.net/viewvc?view=revision&revision=341033>

Test script:
---------------
<?php

$db = new SQLite3(':memory:');
$db->exec("CREATE TABLE foo (num int)");
$db->exec("INSERT INTO foo VALUES (0)");
$db->exec("INSERT INTO foo VALUES (1)");
$stmt = $db->prepare("SELECT * FROM foo WHERE NUM = ?");
$stmt->bindValue(1, 0, SQLITE3_INTEGER);
$res1 = $stmt->execute();
while ($row = $res1->fetchArray(SQLITE3_ASSOC)) {
    var_dump($row);
}
$stmt->clear();
$stmt->reset();
$res1->finalize();
echo "finished first pass\n";
$stmt->bindValue(1, 1, SQLITE3_INTEGER);
$res2 = $stmt->execute();
while ($row = $res2->fetchArray(SQLITE3_ASSOC)) {
    var_dump($row);
    unset($res1);
}


Expected result:
----------------
array(1) {
  ["num"]=>
  int(0)
}
finished first pass
array(1) {
  ["num"]=>
  int(1)
}


Actual result:
--------------
array(1) {
  ["num"]=>
  int(0)
}
finished first pass
array(1) {
  ["num"]=>
  int(1)
}
array(1) {
  ["num"]=>
  int(1)
}


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-11-15 18:53 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2016-11-16 11:14 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=eb570294a289b45d0dd38efc71065d6b0d314c4b
Log: Fix #73530: Unsetting result set may reset other result set
 [2016-11-16 11:14 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 [2016-11-22 13:14 UTC] krakjoe@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=eb570294a289b45d0dd38efc71065d6b0d314c4b
Log: Fix #73530: Unsetting result set may reset other result set
 [2016-12-29 03:40 UTC] jcrews at gridlox dot net
The change referenced in this bug introduced a regression compared to earlier releases.

Working: php-7.0.13
Fails: php-7.0.14

Description
----------------
Using php-7.0.14 with phpoffice/phpexcel with sqlite3 caching causes documents to fail to load. This is ultimately caused by phpexcel not using SQLite3Result::finalize (or reset) internally, within limited-lifetime functions. This is a significant change in behavior that may break deployed applications during system maintenance.

sqlite3_step has two effects:
- On a reset (or new) statement, the prepared statement will be executed, and the first row will be available.
- On a statement VM that previously had sqlite3_step called, the next result row will be available, until the end of data is reached.
sqlite3_reset resets the statement for execution, not any bound parameters. This also explains the original test script behavior: A step is peformed after a reset, which was implicitly called through unset. The statement was thus re-run, returning the first result, again.

When fetchArray hits SQLITE_DONE, the statement isn't reset, and it shouldn't be. Otherwise, any while(haveData) loops would run forever.
It appears, via static analysis, the correct solution is to sqlite3_reset and sqlite3_clear prior to re-binding parameters in sqlite3stmt::execute (in addition to this change). This will guarantee that each call to execute provides consistent results.

Unfortunately, I do not have sufficient resources to build and test php at this time.

Test Script
----------------
<?php
$db = new SQLite3(':memory:');
$db->exec("CREATE TABLE foo (num int)");
$db->exec("INSERT INTO foo VALUES (0)");
$db->exec("INSERT INTO foo VALUES (1)");
$stmt = $db->prepare("SELECT * FROM foo WHERE NUM = ?");

function pass1($db, $stmt) {
   $stmt->bindValue(1, 0, SQLITE3_INTEGER);
   $res1 = $stmt->execute();
   while ($row = $res1->fetchArray(SQLITE3_ASSOC)) {
       var_dump($row);
   }
   echo "finished first pass\n";
}

function pass2($db, $stmt) {
   $stmt->bindValue(1, 1, SQLITE3_INTEGER);
   $res2 = $stmt->execute(); /* Oops, we forgot to reset in fetchArray, no effect */
   while ($row = $res2->fetchArray(SQLITE3_ASSOC)) {
      var_dump($row);
   }
}

pass1($db, $stmt);
pass2($db, $stmt);
?>

Expected Result (>>php-7.0.13):
----------------
sqlitetest.php:12:
array(1) {
  'num' =>
  int(0)
}
finished first pass
sqlitetest.php:21:
array(1) {
  'num' =>
  int(1)
}

Observed Result:
----------------

sqlitetest.php:12:
array(1) {
  'num' =>
  int(0)
}
finished first pass
sqlitetest.php:21:
array(1) {
  'num' =>
  int(0)
}
 [2016-12-29 11:56 UTC] cmb@php.net
-Status: Closed +Status: Re-Opened
 [2016-12-29 11:56 UTC] cmb@php.net
> The change referenced in this bug introduced a regression
> compared to earlier releases.

Thanks for reporting!

> It appears, via static analysis, the correct solution is to
> sqlite3_reset and sqlite3_clear prior to re-binding parameters
> in sqlite3stmt::execute (in addition to this change).

That doesn't appear to solve the issue with your supplied test
script. I'm not yet sure why, but probably it hasn't been a good
idea to try to resolve this issue in a patch release anyway, so
I'm going to revert the fix for now.
 [2016-12-29 13:03 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=2ba3b275948050ce600c5234b66e840b640ca5a5
Log: Revert &quot;Fix #73530: Unsetting result set may reset other result set&quot;
 [2016-12-29 13:03 UTC] cmb@php.net
-Status: Re-Opened +Status: Closed
 [2016-12-29 13:05 UTC] cmb@php.net
-Status: Closed +Status: Re-Opened
 [2016-12-29 15:19 UTC] jcrews at gridlox dot net
>Thanks for reporting!
>
>> It appears, via static analysis, the correct solution is to
>> sqlite3_reset and sqlite3_clear prior to re-binding parameters
>> in sqlite3stmt::execute (in addition to this change).
>
>That doesn't appear to solve the issue with your supplied test
>script. I'm not yet sure why, but probably it hasn't been a good
>idea to try to resolve this issue in a patch release anyway, so
>I'm going to revert the fix for now.

I will be a good OSS citizen and work on a solution for 7.1.x.
Thank you for reverting in the interim.
 [2017-01-13 10:25 UTC] nikolay dot petkov at peri dot de
Working: PHP 5.6.28
Fails: PHP 5.6.29

I have encountered the same issue with one of our deployed applications that uses the PHPOffice/PHPExcel library to generate Excel part lists.
After the PHP has been updated on the server, the application crashes due to the bug fix and the external library.

Please keep in mind that this is also an issue on 5.6.xx and not only on 7.0.xx/7.1.0!

For now I have to rewrite the application to use another cache method...
 [2017-01-13 10:39 UTC] cmb@php.net
> Please keep in mind that this is also an issue on 5.6.xx and not
> only on 7.0.xx/7.1.0!

The revert of the BC breaking commit is already in PHP 5.6.30RC1,
and as such is supposed to be shipped with PHP 5.6.30.
 [2017-01-20 15:13 UTC] it-solutions at schultz dot ch
I can confirm that PHP 5.6.30 (in my environment) successfully fixed this issue (tested against PHPExcel library using SQLite3 caching method)
Thanks for the work!
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC