Re: PHP 7.0.3 RC1 is available for testing - **** BC break ***

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