PHP 7.0.3 RC1 is available for testing

Hi!

Fedora detected a BC break in 5.6.18RC1 and 7.0.3RC1 related to
session management.

Looks like something to do with last session changes. Yasuo, could you
take a look at it?

--
Stas Malyshev
smalyshev@gmail.com

HI Remi and all,

The reason of this BC I can think of immediately is location of GC.

GC was performed after session read previously. GC is moved before
session read because previous logic is buggy.

Old behavior:

• Do session read (Session data is initialized and store in memory)
• Do session GC. (Previously read session data is deleted because it
  should be removed)
• Obsolete session data is written and alive.

New behavior:

• Do session GC. (All obsolete data is removed)
• Do session read (Obsolete data is gone. Initialize empty $_SESSION)
• Newly created $_SESSION is used and obsolete session data is gone.

I think the reported BC would be fixed by moving GC location in
previous place. In fact, I had to fix phpt by the change.

Old behavior may activate obsolete (should be deleted session). This is wrong.
This could happen in low traffic sites more often.

We have 2 choices

• Enforce new and correct behavior since it will not affect normal
  operation. i.e. Actual production site
• Restore old behavior for the time being. (For this option, I prefer
  to restore only for PHP 5.6, but Ok for PHP 7.0 also. No for master)

Since current session module does not manage session data expiration precisely,
restoring old behavior makes sense as we don't have to/cannot be
strict about session
data management anyway.

Patch for fixing BC would be this, if I'm correct about the cause.

e8f1c29cc96ce333fa808aba126297b77d94abdf (main patch)
57be57ac94ef46fa7a73889a1e7d6e75aa4ab00f (TSRM fix for PHP 5.6)

I appreciate if you could run tests without these patches! Thanks.

Any comments?

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Le 21/01/2016 11:53, Anatol Belski a écrit :

Hi,

PHP 7.0.3 RC1 was just released and can be downloaded from:

Fedora detected a BC break in 5.6.18RC1 and 7.0.3RC1 related to
session management.

This update breaks:

Horde_SessionHandler (2.2.6) and symfony (2.7.9)

https://apps.fedoraproject.org/koschei/package/php-horde-Horde-SessionHandler

• /usr/bin/phpunit .
  PHPUnit 4.8.17 by Sebastian Bergmann and contributors.
  .FE.ESPHP Warning: session_destroy(): Trying to destroy uninitialized
  session in
  /builddir/build/BUILD/php-horde-Horde-SessionHandler-2.2.6/Horde_SessionHandler-2.2.6/test/Horde/SessionHandler/Storage/BuiltinTest.php
  on line 115
  ...........SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS...........
  Time: 231 ms, Memory: 5.00Mb
  There were 2 errors:

1. Horde_SessionHandler_Storage_BuiltinTest::testReopen
   Undefined index: sessiondata
   /builddir/build/BUILD/php-horde-Horde-SessionHandler-2.2.6/Horde_SessionHandler-2.2.6/test/Horde/SessionHandler/Storage/BuiltinTest.php:45
2. Horde_SessionHandler_Storage_BuiltinTest::testDestroy
   session_destroy(): Trying to destroy uninitialized session
   /builddir/build/BUILD/php-horde-Horde-SessionHandler-2.2.6/Horde_SessionHandler-2.2.6/test/Horde/SessionHandler/Storage/BuiltinTest.php:79
   --
   There was 1 failure:
3. Horde_SessionHandler_Storage_BuiltinTest::testRead
   Failed asserting that two strings are equal.
   --- Expected
   +++ Actual
   @@ @@
   -'sessiondata|s:3:"foo";'
   +''
   /builddir/build/BUILD/php-horde-Horde-SessionHandler-2.2.6/Horde_SessionHandler-2.2.6/test/Horde/SessionHandler/Storage/BuiltinTest.php:33
   FAILURES!
   Tests: 36, Assertions: 84, Errors: 2, Failures: 1, Skipped: 37.
   PHP Warning: session_destroy(): Trying to destroy uninitialized
   session in /usr/share/pear/Horde/Cli.php on line 530

• /usr/bin/php -d
  include_path=.:/builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php:/usr/share/php
  /usr/bin/phpunit --exclude-group benchmark,intl-data,tty --bootstrap
  bootstrap.php
  /builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel
  PHPUnit 4.8.17 by Sebastian Bergmann and contributors.
  .............................................FDumpDataCollectorTest.php on
  line 89:
  123
  ................. 63 / 440 ( 14%)
  ............................................................... 126 /
  440 ( 28%)
  ............................................................... 189 /
  440 ( 42%)
  ............................................................... 252 /
  440 ( 57%)
  ............................................................... 315 /
  440 ( 71%)
  ............................................................... 378 /
  440 ( 85%)
  .............SSSSSSSSSSSSSSSSSS...............................
  Time: 17.34 seconds, Memory: 17.00Mb
  There was 1 failure:

Symfony\Component\HttpKernel\Tests\DataCollector\DumpDataCollectorTest::testCollectHtml
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@

• <pre class=sf-dump id=sf-dump data-indent-pad=" "><a

href="test:///builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php:89"
title="/builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php"><span class=sf-dump-meta>DumpDataCollectorTest.php</span></a> on line <span class=sf-dump-meta>89</span>:

• <pre class=sf-dump id=sf-dump data-indent-pad=" "><a href=""

title=""><span class=sf-dump-meta></span></a> on line <span class=sf-dump-meta>89</span>:
<span class=sf-dump-num>123</span>

</pre> /builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php:118 FAILURES! Tests: 746, Assertions: 1310, Failures: 1, Skipped: 18. Legacy deprecation notices (11) >>>>>>>>>>>>>>>>>>>>>>> /builddir/build/BUILDROOT/php-symfony-2.7.9-2.fc24.noarch/usr/share/php/Symfony/Component/Intl + RET=1

Of course, both test suites were OK with 5.6.17.

Remi.

Hi,

-----Original Message-----
From: Remi Collet [mailto:remi@fedoraproject.org]
Sent: Thursday, January 21, 2016 5:20 PM
To: internals@lists.php.net
Subject: Re: [PHP-DEV] PHP 7.0.3 RC1 is available for testing - **** BC break ***

• /usr/bin/php -d
  include_path=.:/builddir/build/BUILDROOT/php-symfony-2.7.9-
  2.fc24.noarch/usr/share/php:/usr/share/php
  /usr/bin/phpunit --exclude-group benchmark,intl-data,tty --bootstrap
  bootstrap.php /builddir/build/BUILDROOT/php-symfony-2.7.9-
  2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel
  PHPUnit 4.8.17 by Sebastian Bergmann and contributors.
  .............................................FDumpDataCollectorTest.php on line 89:
  123
  ................. 63 / 440 ( 14%)
  ............................................................... 126 /
  440 ( 28%)
  ............................................................... 189 /
  440 ( 42%)
  ............................................................... 252 /
  440 ( 57%)
  ............................................................... 315 /
  440 ( 71%)
  ............................................................... 378 /
  440 ( 85%)
  .............SSSSSSSSSSSSSSSSSS...............................
  Time: 17.34 seconds, Memory: 17.00Mb
  There was 1 failure:

Symfony\Component\HttpKernel\Tests\DataCollector\DumpDataCollectorTest::
testCollectHtml
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@

• <pre class=sf-dump id=sf-dump data-indent-pad=" "><a

href="test:///builddir/build/BUILDROOT/php-symfony-2.7.9-
2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataColle
ctor/DumpDataCollectorTest.php:89"
title="/builddir/build/BUILDROOT/php-symfony-2.7.9-
2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataColle
ctor/DumpDataCollectorTest.php"><span class=sf-dump-meta>DumpDataCollectorTest.php</span></a> on line <span class=sf-dump-meta>89</span>:

• <pre class=sf-dump id=sf-dump data-indent-pad=" "><a href=""

title=""><span class=sf-dump-meta></span></a> on line <span class=sf-dump-meta>89</span>:
<span class=sf-dump-num>123</span>

</pre> /builddir/build/BUILDROOT/php-symfony-2.7.9- 2.fc24.noarch/usr/share/php/Symfony/Component/HttpKernel/Tests/DataColle ctor/DumpDataCollectorTest.php:118 FAILURES! Tests: 746, Assertions: 1310, Failures: 1, Skipped: 18. Legacy deprecation notices (11) >>>>>>>>>>>>>>>>>>>>>>> /builddir/build/BUILDROOT/php-symfony-2.7.9- 2.fc24.noarch/usr/share/php/Symfony/Component/Intl + RET=1

The fails in Symfony seems to be caused by ff7ed9021cd72a7f82dd4301cdc266afdff458ad, but also the corresponding places in Symfony seems a bit odd with %f format for a string

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php#L259

So that might be caused by both.

Regards

Anatol

Hi Remi and all,

Fedora detected a BC break in 5.6.18RC1 and 7.0.3RC1 related to
session management.

This update breaks:

Horde_SessionHandler (2.2.6) and symfony (2.7.9)

Thank you for notifying issue.
Remi provided reproduceble phpt for this test failure and
found out the cause of test failure. I really appreciate your help!

Anyway, the cause was bug fix for "session_start() returns
TRUE even when failures". Details for this specific case is following.

Newer session cache limiter has php_session_abort()

static int php_session_cache_limiter(TSRMLS_D) /* {{{ */
{
php_session_cache_limiter_t *lim;

if (PS(cache_limiter)[0] == '\0') return 0;
if (PS(session_status) != php_session_active) return -1;

if (SG(headers_sent)) {
    const char *output_start_filename =

php_output_get_start_filename(TSRMLS_C);
int output_start_lineno = php_output_get_start_lineno(TSRMLS_C);

    php_session_abort(TSRMLS_C);   <<<<<<<<<< This is the cause
    if (output_start_filename) {
        php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot send

session cache limiter - headers already sent (output started at
%s:%d)", output_start_filename, output_start_lineno);
} else {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot send
session cache limiter - headers already sent");
}
return -2;
}

This is added because when session cannot be started, then it should fail.
This fix is related to https://bugs.php.net/bug.php?id=71243
The php_session_abort() is not directly related to this bug, but this (and
other fixes) is added because session_start() returns TRUE even when it fails/
should fail.

Note: PHP 5.6's session_start() return value fix is not perfect to keep
save handler compatibility which is a big one. PHP7 should return FALSE
for session_start() failures always by the fix.

Fixing the broken test should be just removing the php_session_abort() from
php_session_cache_limiter().

The reason why it aborts is because it does not make sense to create and/or
write session data even when client does not get the session ID.
Note: session ID cookie is sent only when it is needed. i.e. New session.

I think it does not matter much for session module & users if we remove
the php_session_abort() or not. PHP 5.6's session_start() returns TRUE for
session read error which is fatal, anyway. Being strict for this case does
not worth much.

Whichever is perfectly OK to me for PHP 5.6. For PHP 7.0, I would
like to keep stricter behavior, but do not care much.

How we handle this is up to release managers, since this fix is valid one
even if it broke test on failure case.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net