Fixed Bug #71038 session_start() returns TRUE on failure

[yohgaki]

Although I prefer maintaining FAILURE/SUCCESS status by return values, it's possible to use PS(session_status) to return proper status from session_start().

This patch fixes
https://bugs.php.net/bug.php?id=71038

The destructive part is

@@ -530,14 +535,14 @@ static void php_session_initialize(TSRMLS_D) /* {{{ */
        /* Read data */
        php_session_track_init(TSRMLS_C);
        if (PS(mod)->s_read(&PS(mod_data), PS(id), &val, &vallen TSRMLS_CC) == FAILURE) {
+               php_session_abort(TSRMLS_C);
                /* Some broken save handler implementation returns FAILURE for non-existent session ID */
                /* It's better to raise error for this, but disabled error for better compatibility */
-               /*
-               php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Failed to read session data: %s (path: %s)", PS(mod)->s_name, PS(save_path));
-               */
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "Failed to read session data: %s (path: %s)", PS(mod)->s_name, PS(save_path));
+               return;
        }

This breaks save handlers returns broken return value (false/FAILURE) for successful read.
Note: Reading non-existing session data (new session ID data) must be SUCCESS because there is no error at all and must return true/SUCCESS.

This change is BC for PHP 5.6, but not for PHP7 since it does not allow broken return value from PS(mod)->s_read.

For user save handlers using file_get_contents(), the only required change is cast to string like test files.

diff --git a/ext/session/tests/session_set_save_handler_class_002.phpt b/ext/session/tests/session_set_save_handler_class_002.phpt
index 6fb831f..faf0adf 100644
--- a/ext/session/tests/session_set_save_handler_class_002.phpt
+++ b/ext/session/tests/session_set_save_handler_class_002.phpt
@@ -34,7 +34,7 @@ class MySession2 extends SessionHandler {
        }

        public function read($id) {
-               return @file_get_contents($this->path . $id);
+               return (string)@file_get_contents($this->path . $id);
        }

        public function write($id, $data) {

Question is if we should fix this bug in PHP 5.6 or not.
Other possible option is ignore s_read() error at all in PHP 5.6 to be compatible.
If nobody cares, I would like to choose 2nd (ignore read errors) option for PHP 5.6.
Any comments?

Note: One fpm test fails, but it should be irrelevant.

[yohgaki]

I've pushed the change to repo directly.