Followup fix to custom session save handlers

2d9885c introduced some regressions.  This addresses those.

  * Don't throw return type notice or session write failure when in an exception
  * Fix tests to properly return true/false since null is no longer falsy/successy
  * Rerecord a few tests to accomodate difference in raised warnings

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index 6720987580925068a42548f3652b31ee4d5eb4d6..1eabc9432eaf291f07269f0bc8d49cbb092d8e7f 100644 (file)
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -79,7 +79,10 @@ static zval *ps_call_handler(zval *func, int argc, zval **argv TSRMLS_DC)
                        /* BC for clever users - Deprecate me */ \
                        ret = SUCCESS; \
                } else { \
-                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Session callback expects true/false return value"); \
+                       if (!EG(exception)) { \
+                               php_error_docref(NULL TSRMLS_CC, E_WARNING, \
+                                                "Session callback expects true/false return value"); \
+                       } \
                        ret = FAILURE; \
                } \
                zval_ptr_dtor(&retval); \

diff --git a/ext/session/session.c b/ext/session/session.c
index d6568bdbb5469b07ff2fb8005b6a50fea91b2a23..05fa95d4da07c2041d09a02c7b95f917815c4134 100644 (file)
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -566,7 +566,7 @@ static void php_session_save_current_state(TSRMLS_D) /* {{{ */
                        }
                }
 
-               if (ret == FAILURE) {
+               if ((ret == FAILURE) && !EG(exception)) {
                        php_error_docref(NULL TSRMLS_CC, E_WARNING, "Failed to write session data (%s). Please "
                                        "verify that the current setting of session.save_path "
                                        "is correct (%s)",

diff --git a/ext/session/tests/bug60634_error_1.phpt b/ext/session/tests/bug60634_error_1.phpt
index e41592f18dd095ef1a5a08890724208291a536ca..cc50e812094608cbbf799633519b41b69f4e6d8a 100644 (file)
--- a/ext/session/tests/bug60634_error_1.phpt
+++ b/ext/session/tests/bug60634_error_1.phpt
@@ -16,6 +16,7 @@ function open($save_path, $session_name) {
 
 function close() {
        echo "close: goodbye cruel world\n";
+       return true;
 }
 
 function read($id) {

diff --git a/ext/session/tests/bug60634_error_2.phpt b/ext/session/tests/bug60634_error_2.phpt
index 7c50948ba837b9032c5e7b71aae961abb7653480..96c8eb61e51e2ec067d29e12817afbe3ab64394c 100644 (file)
--- a/ext/session/tests/bug60634_error_2.phpt
+++ b/ext/session/tests/bug60634_error_2.phpt
@@ -16,6 +16,7 @@ function open($save_path, $session_name) {
 
 function close() {
        echo "close: goodbye cruel world\n";
+       return true;
 }
 
 function read($id) {

diff --git a/ext/session/tests/save_handler.inc b/ext/session/tests/save_handler.inc
index ebe9cac564d6372f69795bf571a850435ca5a75e..d271748259147b86606187e5f7cd53478f026a6e 100644 (file)
--- a/ext/session/tests/save_handler.inc
+++ b/ext/session/tests/save_handler.inc
@@ -31,7 +31,7 @@ function write($id, $session_data) {
     if ($fp = fopen($session_file, "w")) {
         $return = fwrite($fp, $session_data);
         fclose($fp);
-        return $return;
+        return (bool)$return;
     }
     return false;
 }
@@ -40,7 +40,8 @@ function destroy($id) {
     global $session_save_path, $name;
     echo "Destroy [${session_save_path},${id}]\n";
     $session_file = "$session_save_path/".SESSION_FILE_PREFIX.$id;
-    return unlink($session_file);
+    unlink($session_file);
+    return true;
 }
 
 function gc($maxlifetime) {

diff --git a/ext/session/tests/session_module_name_variation3.phpt b/ext/session/tests/session_module_name_variation3.phpt
index dc1c6ba5ec58f05d2ce1fb98fff09983676f2fca..e32790d54eff35326a92af2760d52f63c9585b0f 100644 (file)
--- a/ext/session/tests/session_module_name_variation3.phpt
+++ b/ext/session/tests/session_module_name_variation3.phpt
@@ -21,11 +21,11 @@ function open($save_path, $session_name) {
     throw new Exception("Stop...!");
 }
 
-function close() { }
-function read($id) { }
-function write($id, $session_data) { }
-function destroy($id) { }
-function gc($maxlifetime) { }
+function close() { return true; }
+function read($id) { return ''; }
+function write($id, $session_data) { return true; }
+function destroy($id) { return true; }
+function gc($maxlifetime) { return true; }
 
 var_dump(session_module_name("files"));
 session_set_save_handler("open", "close", "read", "write", "destroy", "gc");
@@ -41,9 +41,11 @@ ob_end_flush();
 string(%d) "%s"
 string(4) "user"
 
-Fatal error: Uncaught exception 'Exception' with message 'Stop...!' in %s:%d
+Warning: Uncaught exception 'Exception' with message 'Stop...!' in %s:%d
 Stack trace:
 #0 [internal function]: open('', 'PHPSESSID')
 #1 %s(%d): session_start()
 #2 {main}
   thrown in %s on line %d
+
+Fatal error: session_start(): Failed to initialize storage module: %s in %s/session_module_name_variation3.php on line %d

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 6fb831f6957a16ca638f3de74057d6d513ddb501..b75a7e63908982b05d4fa4a22755291ac0feb57e 100644 (file)
--- a/ext/session/tests/session_set_save_handler_class_002.phpt
+++ b/ext/session/tests/session_set_save_handler_class_002.phpt
@@ -38,11 +38,12 @@ class MySession2 extends SessionHandler {
        }
 
        public function write($id, $data) {
-               return file_put_contents($this->path . $id, $data);
+               return (bool)file_put_contents($this->path . $id, $data);
        }
 
        public function destroy($id) {
                @unlink($this->path . $id);
+               return true;
        }
 
        public function gc($maxlifetime) {

diff --git a/ext/session/tests/session_set_save_handler_class_005.phpt b/ext/session/tests/session_set_save_handler_class_005.phpt
index c74c81de1dbc8da8d792fbfd86098fa377e5d921..5be735306ac6a8b6092c576afb788d8c5ed856c3 100644 (file)
--- a/ext/session/tests/session_set_save_handler_class_005.phpt
+++ b/ext/session/tests/session_set_save_handler_class_005.phpt
@@ -52,4 +52,6 @@ array(0) {
 
 Warning: SessionHandler::write(): Parent session handler is not open in %ssession_set_save_handler_class_005.php on line %d
 
+Warning: session_write_close(): Failed to write session data %s in %ssession_set_save_handler_class_005.php on line %d
+
 Warning: SessionHandler::close(): Parent session handler is not open in %ssession_set_save_handler_class_005.php on line %d

diff --git a/ext/session/tests/session_set_save_handler_class_012.phpt b/ext/session/tests/session_set_save_handler_class_012.phpt
index 3899d28816e19856d9c4030a173b017235fd9798..60cd12955c928e88ef7dc0781e5e4fecf055f7b7 100644 (file)
--- a/ext/session/tests/session_set_save_handler_class_012.phpt
+++ b/ext/session/tests/session_set_save_handler_class_012.phpt
@@ -24,7 +24,9 @@ class MySession extends SessionHandler {
        public function open($path, $name) {
                ++$this->i;
                echo 'Open ', session_id(), "\n";
-               return parent::open();
+               // This test was written for broken return value handling
+               // Mimmick what was actually being tested by returning true here
+               return (null === parent::open());
        }
        public function read($key) {
                ++$this->i;
@@ -57,4 +59,6 @@ array(0) {
 
 Warning: Unknown: Parent session handler is not open in Unknown on line 0
 
+Warning: Unknown: Failed to write session data %s in %s on line %d
+
 Warning: Unknown: Parent session handler is not open in Unknown on line 0

diff --git a/ext/session/tests/session_set_save_handler_class_016.phpt b/ext/session/tests/session_set_save_handler_class_016.phpt
index 2de03c0682e56d59520e820a962fa3d28eccaf24..521bd86f31845380106f39291ceb5aa131eaa175 100644 (file)
--- a/ext/session/tests/session_set_save_handler_class_016.phpt
+++ b/ext/session/tests/session_set_save_handler_class_016.phpt
@@ -38,7 +38,7 @@ class MySession2 extends SessionHandler {
        }
 
        public function write($id, $data) {
-               return file_put_contents($this->path . $id, $data);
+               return (bool)file_put_contents($this->path . $id, $data);
        }
 
        public function destroy($id) {

diff --git a/ext/session/tests/session_set_save_handler_error3.phpt b/ext/session/tests/session_set_save_handler_error3.phpt
index cb07b0d8dec1e6a4632cd462eb814a776ff0c713..859ba11ad53c36463d58cef8b4fc3763bb1ffdd0 100644 (file)
--- a/ext/session/tests/session_set_save_handler_error3.phpt
+++ b/ext/session/tests/session_set_save_handler_error3.phpt
@@ -34,9 +34,11 @@ ob_end_flush();
 --EXPECTF--
 *** Testing session_set_save_handler() : error functionality ***
 
-Fatal error: Uncaught exception 'Exception' with message 'Do something bad..!' in %s:%d
+Warning: Uncaught exception 'Exception' with message 'Do something bad..!' in %s:%d
 Stack trace:
 #0 [internal function]: open('', 'PHPSESSID')
 #1 %s(%d): session_start()
 #2 {main}
   thrown in %s on line %d
+
+Fatal error: session_start(): Failed to initialize storage module: %s in %ssession_set_save_handler_error3.php on line %d

diff --git a/ext/session/tests/session_set_save_handler_error4.phpt b/ext/session/tests/session_set_save_handler_error4.phpt
index d286f07d99d7ff411061fbce8690ac98678c34b2..be3429b084240b864f7246e09c32d96c7826f0d6 100644 (file)
--- a/ext/session/tests/session_set_save_handler_error4.phpt
+++ b/ext/session/tests/session_set_save_handler_error4.phpt
@@ -15,7 +15,7 @@ ob_start();
 
 echo "*** Testing session_set_save_handler() : error functionality ***\n";
 
-function callback() { }
+function callback() { return true; }
 
 session_set_save_handler("callback", "callback", "callback", "callback", "callback", "callback");
 session_set_save_handler("callback", "echo", "callback", "callback", "callback", "callback");

diff --git a/ext/session/tests/session_set_save_handler_iface_001.phpt b/ext/session/tests/session_set_save_handler_iface_001.phpt
index 39a4b9975b58966e99596d49fa431b7b1d331878..03ee42865c093cb425dbc67fa5511ebbb9a99056 100644 (file)
--- a/ext/session/tests/session_set_save_handler_iface_001.phpt
+++ b/ext/session/tests/session_set_save_handler_iface_001.phpt
@@ -38,7 +38,7 @@ class MySession2 implements SessionHandlerInterface {
        }
 
        public function write($id, $data) {
-               return file_put_contents($this->path . $id, $data);
+               return (bool)file_put_contents($this->path . $id, $data);
        }
 
        public function destroy($id) {

diff --git a/ext/session/tests/session_set_save_handler_variation4.phpt b/ext/session/tests/session_set_save_handler_variation4.phpt
index 3485f2373aa5feac448c34b39bce469a6021a6bf..c34eb9cd9fb041f03283d620483fbed0774c1c47 100644 (file)
--- a/ext/session/tests/session_set_save_handler_variation4.phpt
+++ b/ext/session/tests/session_set_save_handler_variation4.phpt
@@ -24,6 +24,7 @@ echo "*** Testing session_set_save_handler() : variation ***\n";
 function noisy_gc($maxlifetime) {
        echo("GC [".$maxlifetime."]\n");
        gc($maxlifetime);
+       return true;
 }
 
 require_once "save_handler.inc";