On Jan 26, 2016 4:02 AM, "Yasuo Ohgaki" <yohg...@ohgaki.net> wrote:
>
> Hi Remi and all,
>
> On Fri, Jan 22, 2016 at 1:20 AM, Remi Collet <r...@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
