SQLite 3.14

php.internals

Christoph Becker

10 years ago
Hi! SQLite 3.14 has been released today. It brings some improvements and new features, and fixes a few bugs, see <http://www.sqlite.org/releaselog/3_14.html>. I think PHP-7.1 should be updated, but I'm not sure about PHP-7.0, even though I found no regression when running the sqlite3 and pdo_sqlite test suites with 3.14 on Linux and Windows under a current PHP-7.0. Anatol, what do you think?
-- Christoph M. Becker

Anatol Belski

10 years ago
Hi Christoph,
> -----Original Message----- > From: Christoph M. Becker [mailto:cmbecker69@gmx.de] > Sent: Monday, August 8, 2016 11:23 PM > To: internals@lists.php.net; Anatol Belski <anatol.php@belski.net> > Subject: [PHP-DEV] SQLite 3.14 > > Hi! > > SQLite 3.14 has been released today. It brings some improvements and new > features, and fixes a few bugs, see > <http://www.sqlite.org/releaselog/3_14.html>. > > I think PHP-7.1 should be updated, but I'm not sure about PHP-7.0, even though I > found no regression when running the sqlite3 and pdo_sqlite test suites with > 3.14 on Linux and Windows under a current PHP-7.0. > > Anatol, what do you think? >
As long as 7.1 is still in pre cycle, it'd be cheap to upgrade there first. If there are no issues, we're safer to backport this SQLite version into a lower PHP branch. Thanks Anatol

Christoph Becker

10 years ago
On 10.08.2016 at 16:01, Anatol Belski wrote:
> Hi Christoph, > >> -----Original Message----- >> From: Christoph M. Becker [mailto:cmbecker69@gmx.de] >> Sent: Monday, August 8, 2016 11:23 PM >> To: internals@lists.php.net; Anatol Belski <anatol.php@belski.net> >> Subject: [PHP-DEV] SQLite 3.14 >> >> Hi! >> >> SQLite 3.14 has been released today. It brings some improvements and new >> features, and fixes a few bugs, see >> <http://www.sqlite.org/releaselog/3_14.html>. >> >> I think PHP-7.1 should be updated, but I'm not sure about PHP-7.0, even though I >> found no regression when running the sqlite3 and pdo_sqlite test suites with >> 3.14 on Linux and Windows under a current PHP-7.0. >> >> Anatol, what do you think? >> > As long as 7.1 is still in pre cycle, it'd be cheap to upgrade there first. If there are no issues, we're safer to backport this SQLite version into a lower PHP branch.
Sounds reasonable, Anotol. Thanks. So I'm going to update 7.1+ ASAP.
-- Christoph M. Becker

Rasmus Schultz

10 years ago
On Wed, Aug 10, 2016 at 4:08 PM, Christoph M. Becker <cmbecker69@gmx.de> wrote:
> Sounds reasonable, Anotol. Thanks. So I'm going to update 7.1+ ASAP.
Are you going to include any plugins? I noticed today, SQLite has interesting optional features - in particular, the "json1" plug-in looks interesting. Both MySQL and Postgres have JSON features now, so it would be nice if the supported file-database also had this feature.

Christoph Becker

10 years ago
On 10.08.2016 at 20:51, Rasmus Schultz wrote:
> On Wed, Aug 10, 2016 at 4:08 PM, Christoph M. Becker <cmbecker69@gmx.de> wrote: > >> Sounds reasonable, Anotol. Thanks. So I'm going to update 7.1+ ASAP. > > Are you going to include any plugins? > > I noticed today, SQLite has interesting optional features - in > particular, the "json1" plug-in looks interesting. > > Both MySQL and Postgres have JSON features now, so it would be nice if > the supported file-database also had this feature.
I'm not aware that there are any plans to bundle some of the plugins[1], but as I understand it, that's not really necessary, as (most?) plugins can be loaded as shared extensions. And in practise, most installations probably don't use the bundled SQLite anyway. [1] There is a feature request to support the online backup API (<https://bugs.php.net/bug.php?id=70950>), but that would require additional PHP bindings, I presume.
-- Christoph M. Becker

Davey

10 years ago
Hey Christoph, I saw this failure while packaging 7.1.0beta3, and assume it might be related to your update: FAIL SQLite3 open_basedir checks [ext/sqlite3/tests/sqlite3_21_security.phpt] ========DIFF======== 006- 007- Warning: SQLite3::__construct(): open_basedir restriction in effect. File(%s) is not within the allowed path(s): (.) in %ssqlite3_21_security.php on line %d 008- Exception: open_basedir prohibits opening %s in %ssqlite3_21_security.php:%d 009- Stack trace: 010- #0 %ssqlite3_21_security.php(%d): SQLite3->__construct('%s') 011- #1 {main} ========DONE======== Can you please look into this in time for RC1? Thanks, - Davey On Thu, Aug 11, 2016 at 12:08 AM, Christoph M. Becker <cmbecker69@gmx.de> wrote:

Christoph Becker

10 years ago
Hi Davey! On 19.08.2016 at 15:32, Davey Shafik wrote:
> I saw this failure while packaging 7.1.0beta3, and assume it might be > related to your update: > > FAIL SQLite3 open_basedir checks > [ext/sqlite3/tests/sqlite3_21_security.phpt] > > ========DIFF======== > 006- > 007- Warning: SQLite3::__construct(): open_basedir restriction in effect. > File(%s) is not within the allowed path(s): (.) in > %ssqlite3_21_security.php on line %d > 008- Exception: open_basedir prohibits opening %s in > %ssqlite3_21_security.php:%d > 009- Stack trace: > 010- #0 %ssqlite3_21_security.php(%d): SQLite3->__construct('%s') > 011- #1 {main} > ========DONE======== > > Can you please look into this in time for RC1?
I've just checked again with the tagged PHP-7.1.0beta3, but the test succeeds on my machine. Therefore it's hard for me to assess what is wrong. According to the diff, it appears that the second DB which shouldn't be created according to the open_basedir restriction, is actually successfully created. Anyway, it's rather unlikely that an open_basedir related failure is caused by updating SQLite, as this check is part of the PHP binding[1], which has not been affected by this commit. The issue might be caused by commit cc125f27[2], but that's also somewhat unlikely, because the Travis checks usually succeed generally. Can you reproduce the test failure? In which enviroment? [1] <https://github.com/php/php-src/blob/PHP-7.1.0beta3/ext/sqlite3 /sqlite3.c#L125> [2] <https://github.com/php/php-src/commit/cc125f27>
-- Christoph M. Becker

Davey

10 years ago
Christophe, I got the failure multiple times in my Debian Jessie docker container that I use for builds - you can check it out yourself at https://github.com/dshafik/php-build to see the setup. Thanks for looking into this! - Davey On Sat, Aug 20, 2016 at 01:35 Christoph M. Becker <cmbecker69@gmx.de> wrote:

Davey

10 years ago
Argh! Apologies for autocomplete adding that 'e' to your name! - Davey On Sat, Aug 20, 2016 at 03:09 Davey Shafik <davey@php.net> wrote:

Christoph Becker

10 years ago
On 19.08.2016 at 19:09, Davey Shafik wrote:
> I got the failure multiple times in my Debian Jessie docker container that > I use for builds - you can check it out yourself at > https://github.com/dshafik/php-build to see the setup. > > Thanks for looking into this!
Of course! And thanks for the link to your docker container. I've tried to reproduce the test failure in a similar environment, but the test always passed. I've also reviewed the latest commits to ext/sqlite3/sqlite3.c, but I couldn't find anything that might have caused the regression. I'm at a loss – would it be possible to augment the test with some hopefully helpful diagnostic messages and a check whether touch()ing this filename would fail? I mean something like this: ext/sqlite3/tests/sqlite3_21_security.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/sqlite3/tests/sqlite3_21_security.phpt b/ext/sqlite3/tests/sqlite3_21_security.phpt index f28c736..b8962a8 100644 --- a/ext/sqlite3/tests/sqlite3_21_security.phpt +++ b/ext/sqlite3/tests/sqlite3_21_security.phpt @@ -18,6 +18,8 @@ unlink($directory . $file); echo "Above test directory\n"; try { $db = new SQLite3('../bad' . $file); + var_dump($db, getcwd(), $file, realpath('../bad' . $file)); + touch('../bad' . $file); } catch (Exception $e) { echo $e . "\n"; } If it would be too much work for you to test a modified version, this patch might be committed to PHP 7.0 (the code is dead code anyway, if the test succeeds), and maybe removed later. BTW: do you deliberately run the tests without -n in <https://github.com/dshafik/php-build/blob/master/build.sh#L294>? Regards, Christoph

Anatol Belski

10 years ago
Hi Davey,
> -----Original Message----- > From: Davey Shafik [mailto:davey@php.net] > Sent: Friday, August 19, 2016 7:09 PM > To: Christoph M. Becker <cmbecker69@gmx.de> > Cc: Anatol Belski <anatol.php@belski.net>; internals@lists.php.net > Subject: Re: [PHP-DEV] SQLite 3.14 > > Christophe, > > I got the failure multiple times in my Debian Jessie docker container that I use > for builds - you can check it out yourself at https://github.com/dshafik/php-build > to see the setup. > > Thanks for looking into this! > > - Davey > > On Sat, Aug 20, 2016 at 01:35 Christoph M. Becker <cmbecker69@gmx.de > <mailto:cmbecker69@gmx.de> > wrote: > > > Hi Davey! > > On 19.08.2016 at 15:32, Davey Shafik wrote: > > > I saw this failure while packaging 7.1.0beta3, and assume it might be > > related to your update: > > > > FAIL SQLite3 open_basedir checks > > [ext/sqlite3/tests/sqlite3_21_security.phpt] > > > > ========DIFF======== > > 006- > > 007- Warning: SQLite3::__construct(): open_basedir restriction in > effect. > > File(%s) is not within the allowed path(s): (.) in > > %ssqlite3_21_security.php on line %d > > 008- Exception: open_basedir prohibits opening %s in > > %ssqlite3_21_security.php:%d > > 009- Stack trace: > > 010- #0 %ssqlite3_21_security.php(%d): SQLite3->__construct('%s') > > 011- #1 {main} > > ========DONE======== > > > > Can you please look into this in time for RC1? > > I've just checked again with the tagged PHP-7.1.0beta3, but the test > succeeds on my machine. Therefore it's hard for me to assess what is > wrong. According to the diff, it appears that the second DB which > shouldn't be created according to the open_basedir restriction, is > actually successfully created. > > Anyway, it's rather unlikely that an open_basedir related failure is > caused by updating SQLite, as this check is part of the PHP binding[1], > which has not been affected by this commit. The issue might be caused > by commit cc125f27[2], but that's also somewhat unlikely, because the > Travis checks usually succeed generally. > > Can you reproduce the test failure? In which enviroment? > > [1] <https://github.com/php/php-src/blob/PHP-7.1.0beta3/ext/sqlite3 > /sqlite3.c#L125 <https://github.com/php/php-src/blob/PHP- > 7.1.0beta3/ext/sqlite3/sqlite3.c#L125> > > [2] <https://github.com/php/php-src/commit/cc125f27> >
I was checking this and saw no failure as well. From the test diff, it doesn't look like a crash but something with that try/catch block. Maybe there'll be more luck with a reproducer, if you could post the exact command line? You can read it from the corresponding .sh file or just by appending --verbose. Regards Anatol

Davey

10 years ago
Sorry, I dropped the ball on this one: ./sapi/cli/php -d "output_handler=" -d "open_basedir=." -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=1" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "memory_limit=128M" -d "log_errors_max_len=0" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -f "/php-src/ext/sqlite3/tests/sqlite3_21_security.php" I think the issue is that the test isn't run in ext/sqlite3/tests, but from the root of the checkout, which means that open_basedir=. would include everything in the repo, including the attempt "../bad" directory. Potential solutions: Change the path to be "../../../../../bad" to ensure it's outside the top-level of the script. Add a: chdir(__DIR__); at the top. Thoughts? On Fri, Aug 19, 2016 at 5:31 PM, Anatol Belski <anatol.php@belski.net> wrote:

Christoph Becker

10 years ago
On 31.08.2016 at 05:46, Davey Shafik wrote:
> Sorry, I dropped the ball on this one: > > ../sapi/cli/php -d "output_handler=" -d "open_basedir=." -d > "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" > -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d > "html_errors=0" -d "track_errors=1" -d "report_memleaks=1" -d > "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d > "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" > -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d > "memory_limit=128M" -d "log_errors_max_len=0" -d "opcache.fast_shutdown=0" > -d "opcache.file_update_protection=0" -f > "/php-src/ext/sqlite3/tests/sqlite3_21_security.php" > > I think the issue is that the test isn't run in ext/sqlite3/tests, but from > the root of the checkout, which means that open_basedir=. would include > everything in the repo, including the attempt "../bad" directory.
Ah, I have not thought of running in a root directory directly (or would have expected that "../bad" would still trigger the open_basedir warning).
> Potential solutions: > > Change the path to be "../../../../../bad" to ensure it's outside the > top-level of the script.
If "../bad" doesn't trigger the open_basedir restriction, "../../bad" most likely also wouldn't. Please correct me if I'm wrong.
> Add a: chdir(__DIR__); at the top.
Indeed, using chdir() seems to be the proper way to test for open_basedir restrictions, see <https://github.com/php/php-src/blob/PHP-7.0.11/tests/security/open_basedir.inc#L12-L14>. Should that be changed for PHP-5.6+? Cheers!

Davey

10 years ago
Good find with that other test! I don't see any reason not to fix it for 5.6+ - Davey On Fri, Sep 2, 2016 at 12:08 PM, Christoph M. Becker <cmbecker69@gmx.de> wrote: