Merge branch 'PHP-5.6' into PHP-7.0

* PHP-5.6:
  Fixed bug #69111 (Crash in SessionHandler::read()). Made session save handler abuse much harder than before.

9 files changed, 104 insertions, 7 deletions

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index e6f162855a..c7c09ff781 100644
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -85,7 +85,16 @@ PS_OPEN_FUNC(user)
 	ZVAL_STRING(&args[0], (char*)save_path);
 	ZVAL_STRING(&args[1], (char*)session_name);
 
-	ps_call_handler(&PSF(open), 2, args, &retval);
+	zend_try {
+		ps_call_handler(&PSF(open), 2, args, &retval);
+	} zend_catch {
+		PS(session_status) = php_session_none;
+		if (!Z_ISUNDEF(retval)) {
+			zval_ptr_dtor(&retval);
+		}
+		zend_bailout();
+	} zend_end_try();
+
 	PS(mod_user_implemented) = 1;
 
 	FINISH;
diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c
index 59b44f5f6f..a774d4bf9c 100644
--- a/ext/session/mod_user_class.c
+++ b/ext/session/mod_user_class.c
@@ -22,6 +22,10 @@
 #include "php_session.h"
 
 #define PS_SANITY_CHECK						\
+	if (PS(session_status) != php_session_active) { \
+		php_error_docref(NULL, E_WARNING, "Session is not active"); \
+		RETURN_FALSE; \
+	} \
 	if (PS(default_mod) == NULL) {				\
 		php_error_docref(NULL, E_CORE_ERROR, "Cannot call default session handler"); \
 		RETURN_FALSE;						\
@@ -40,6 +44,7 @@ PHP_METHOD(SessionHandler, open)
 {
 	char *save_path = NULL, *session_name = NULL;
 	size_t save_path_len, session_name_len;
+	int ret;
 
 	PS_SANITY_CHECK;
 
@@ -48,7 +53,15 @@ PHP_METHOD(SessionHandler, open)
 	}
 
 	PS(mod_user_is_open) = 1;
-	RETVAL_BOOL(SUCCESS == PS(default_mod)->s_open(&PS(mod_data), save_path, session_name));
+
+	zend_try {
+		ret = PS(default_mod)->s_open(&PS(mod_data), save_path, session_name);
+	} zend_catch {
+		PS(session_status) = php_session_none;
+		zend_bailout();
+	} zend_end_try();
+
+	RETVAL_BOOL(SUCCESS == ret);
 }
 /* }}} */
 
@@ -56,6 +69,8 @@ PHP_METHOD(SessionHandler, open)
    Wraps the old close handler */
 PHP_METHOD(SessionHandler, close)
 {
+	int ret;
+
 	PS_SANITY_CHECK_IS_OPEN;
 
 	// don't return on failure, since not closing the default handler
@@ -63,7 +78,15 @@ PHP_METHOD(SessionHandler, close)
 	zend_parse_parameters_none();
 
 	PS(mod_user_is_open) = 0;
-	RETVAL_BOOL(SUCCESS == PS(default_mod)->s_close(&PS(mod_data)));
+
+	zend_try {
+		ret = PS(default_mod)->s_close(&PS(mod_data));
+	} zend_catch {
+		PS(session_status) = php_session_none;
+		zend_bailout();
+	} zend_end_try();
+
+	RETVAL_BOOL(SUCCESS == ret);
 }
 /* }}} */
 
diff --git a/ext/session/session.c b/ext/session/session.c
index 4b0643d021..b4a63bd79b 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -103,6 +103,7 @@ static void php_session_abort(void);
 static inline void php_rinit_session_globals(void) /* {{{ */
 {
 	/* Do NOT init PS(mod_user_names) here! */
+	/* TODO: These could be moved to MINIT and removed. These should be initialized by php_rshutdown_session_globals() always when execution is finished. */
 	PS(id) = NULL;
 	PS(session_status) = php_session_none;
 	PS(mod_data) = NULL;
@@ -130,10 +131,15 @@ static inline void php_rshutdown_session_globals(void) /* {{{ */
 		zend_string_release(PS(id));
 		PS(id) = NULL;
 	}
+
 	if (PS(session_vars)) {
 		zend_string_release(PS(session_vars));
 		PS(session_vars) = NULL;
 	}
+
+	/* User save handlers may end up directly here by misuse, bugs in user script, etc. */
+	/* Set session status to prevent error while restoring save handler INI value. */
+	PS(session_status) = php_session_none;
 }
 /* }}} */
 
@@ -1662,8 +1668,8 @@ PHPAPI void php_session_start(void) /* {{{ */
 static void php_session_flush(int write) /* {{{ */
 {
 	if (PS(session_status) == php_session_active) {
-		PS(session_status) = php_session_none;
 		php_session_save_current_state(write);
+		PS(session_status) = php_session_none;
 	}
 }
 /* }}} */
@@ -1671,10 +1677,10 @@ static void php_session_flush(int write) /* {{{ */
 static void php_session_abort(void) /* {{{ */
 {
 	if (PS(session_status) == php_session_active) {
-		PS(session_status) = php_session_none;
 		if (PS(mod_data) || PS(mod_user_implemented)) {
 			PS(mod)->s_close(&PS(mod_data));
 		}
+		PS(session_status) = php_session_none;
 	}
 }
 /* }}} */
diff --git a/ext/session/tests/bug55688.phpt b/ext/session/tests/bug55688.phpt
index 8db48384af..b073dc3c5c 100644
--- a/ext/session/tests/bug55688.phpt
+++ b/ext/session/tests/bug55688.phpt
@@ -12,4 +12,4 @@ $x = new SessionHandler;
 $x->gc(1);
 ?>
 --EXPECTF--
-Warning: SessionHandler::gc(): Parent session handler is not open in %s on line %d
+Warning: SessionHandler::gc(): Session is not active in %s on line %d
diff --git a/ext/session/tests/bug60634.phpt b/ext/session/tests/bug60634.phpt
index 86dcb11526..b2f5076287 100644
--- a/ext/session/tests/bug60634.phpt
+++ b/ext/session/tests/bug60634.phpt
@@ -39,8 +39,17 @@ session_start();
 session_write_close();
 echo "um, hi\n";
 
+/*
+FIXME: Since session module try to write/close session data in
+RSHUTDOWN, write() is executed twices. This is caused by undefined
+function error and zend_bailout(). Current session module codes
+depends on this behavior. These codes should be modified to remove
+multiple write().
+*/
+
 ?>
 --EXPECTF--
 write: goodbye cruel world
+write: goodbye cruel world
 close: goodbye cruel world
 
diff --git a/ext/session/tests/bug60634_error_1.phpt b/ext/session/tests/bug60634_error_1.phpt
index d0733f5a5a..fa76ff522a 100644
--- a/ext/session/tests/bug60634_error_1.phpt
+++ b/ext/session/tests/bug60634_error_1.phpt
@@ -41,6 +41,11 @@ session_start();
 session_write_close();
 echo "um, hi\n";
 
+/*
+FIXME: Something wrong. It should try to close after error, otherwise session
+may keep "open" state.
+*/
+
 ?>
 --EXPECTF--
 write: goodbye cruel world
@@ -51,3 +56,4 @@ Stack trace:
 #1 %s(%d): session_write_close()
 #2 {main}
   thrown in %s on line %d
+
diff --git a/ext/session/tests/bug67972.phpt b/ext/session/tests/bug67972.phpt
index 63ed3a95b8..92c3044ac5 100644
--- a/ext/session/tests/bug67972.phpt
+++ b/ext/session/tests/bug67972.phpt
@@ -7,4 +7,5 @@ Bug #67972: SessionHandler Invalid memory read create_sid()
 
 (new SessionHandler)->create_sid();
 --EXPECTF--
-Fatal error: SessionHandler::create_sid(): Cannot call default session handler in %s on line %d
+Warning: SessionHandler::create_sid(): Session is not active in %s on line %d
+
diff --git a/ext/session/tests/bug69111.phpt b/ext/session/tests/bug69111.phpt
new file mode 100644
index 0000000000..f5def0ed35
--- /dev/null
+++ b/ext/session/tests/bug69111.phpt
@@ -0,0 +1,36 @@
+--TEST--
+Bug #69111 (Crash in SessionHandler::read())
+--INI--
+session.save_path=
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+$sh = new SessionHandler;
+session_set_save_handler($sh);
+
+$savePath = ini_get('session.save_path');
+$sessionName = ini_get('session.name');
+
+// session_start(); // Uncommenting this makes it not crash when reading the session (see below), but it will not return any data.
+
+$sh->open($savePath, $sessionName);
+$sh->write("foo", "bar");
+$sh->read($id);
+$sh->gc(1245);
+$sh->close();
+?>
+--EXPECTF--
+Warning: SessionHandler::open(): Session is not active in %s on line 10
+
+Warning: SessionHandler::write(): Session is not active in %s on line 11
+
+Notice: Undefined variable: id in %s on line 12
+
+Warning: SessionHandler::read(): Session is not active in %s on line 12
+
+Warning: SessionHandler::gc(): Session is not active in %s on line 13
+
+Warning: SessionHandler::close(): Session is not active in %s on line 14
\ No newline at end of file
diff --git a/ext/session/tests/sessionhandler_open_001.phpt b/ext/session/tests/sessionhandler_open_001.phpt
index 6ade9e00a5..e6e913a6a5 100644
--- a/ext/session/tests/sessionhandler_open_001.phpt
+++ b/ext/session/tests/sessionhandler_open_001.phpt
@@ -16,4 +16,11 @@ print "Done!\n";
 
 ?>
 --EXPECTF--
+Warning: SessionHandler::open(): Session is not active in %s on line 5
+
+Warning: SessionHandler::open(): Session is not active in %s on line 6
+
+Warning: SessionHandler::open(): Session is not active in %s on line 7
+
+Warning: SessionHandler::open(): Session is not active in %s on line 8
 Done!