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: 2020-02-22 15:51 UTC
Votes:4
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:0 (0.0%)
From: cmb@php.net Assigned:
Status: Re-Opened Package: SQLite related
PHP Version: master-Git-2016-11-15 (Git) OS: *
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [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!
 [2017-10-24 05:26 UTC] kalle@php.net
-Status: Re-Opened +Status: Assigned
 [2017-10-24 16:41 UTC] cmb@php.net
-Status: Assigned +Status: Re-Opened -Assigned To: cmb +Assigned To:
 [2017-10-24 16:41 UTC] cmb@php.net
Well, I'm not sure whether this could be solved at all.
 [2020-02-22 15:51 UTC] cmb@php.net
An actually related issue:

<?php
$db = new SQLite3(':memory:');

// let's consider this to be already in the db
$db->query("CREATE TABLE dog ( id INTEGER PRIMARY KEY, name TEXT, annoying INTEGER )");
$result = $db->exec("INSERT INTO dog VALUES (1, 'Annoying Dog', 1)");

// process some query
$result = $db->query("SELECT 1");
$result->fetchArray();
unset($result);

// now trigger an error
$result = $db->exec("INSERT INTO dog VALUES (1, 'Annoying Dog', 1)");
var_dump($db->lastErrorCode());
?>

Output:

Warning: SQLite3::exec(): UNIQUE constraint failed: dog.id in %s on line %d
int(19)

That is to be expected; however, if we did not unset($result)
explicitly, the output would be:

Warning: SQLite3::exec(): UNIQUE constraint failed: dog.id in %s on line %d
int(0)

In this case the error code is changed, because the SQLite3Result
of the "SELECT 1" query is destroyed after the "INSERT INTO"
query has been executed, and this triggers the sqlite3_reset()
which causes the DB error to be set to zero.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 09:01:30 2024 UTC