Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:90936
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.47 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXbavNd3H3+c_ASOBH6O6LoaBsj52pxPZmjTKZyssup7gw@mail.gmail.com>
References: <03a501d15439$fcbf9ca0$f63ed5e0$@php.net>
	<56A1054A.5080102@fedoraproject.org>
	<CAGa2bXbavNd3H3+c_ASOBH6O6LoaBsj52pxPZmjTKZyssup7gw@mail.gmail.com>
Date: Tue, 26 Jan 2016 18:35:00 +0700
Message-ID: <CAEZPtU4v1Qm2AzBZcjbMdpgwRh+2o6ZqcV5yT-ov+QkFEH8j_g@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: PHP internals <internals@lists.php.net>, Ferenc Kovacs <tyrael@php.net>, 
	julien pauli <jpauli@php.net>, Remi Collet <remi@fedoraproject.org>
Content-Type: multipart/alternative; boundary=001a1130cf266a4352052a3b13a1
Subject: Re: [PHP-DEV] PHP 7.0.3 RC1 is available for testing - **** BC break ***
From: pierre.php@gmail.com (Pierre Joye)

--001a1130cf266a4352052a3b13a1
Content-Type: text/plain; charset=UTF-8

On Jan 26, 2016 4:02 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote:
>
> Hi Remi and all,
>
> On Fri, Jan 22, 2016 at 1:20 AM, Remi Collet <remi@fedoraproject.org>
wrote:
> > 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().

Fixing broken tests most likely mean BC will remain which is not so good.

I understand the overall goal to improve session security but this is an
area that has behaved this way for years. I am totally convinced that such
big changes should have (or should) in stable branches, be 7.0 or 5.6.
Especially because testing these changes take time.

Cheers,
Pierre

--001a1130cf266a4352052a3b13a1--