reset the ext/session to the state of 5.6.17

14 files changed, 69 insertions, 187 deletions

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index de2df9d6a7..0b6fb626fd 100644
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -91,16 +91,7 @@ PS_OPEN_FUNC(user)
 	SESS_ZVAL_STRING((char*)save_path, args[0]);
 	SESS_ZVAL_STRING((char*)session_name, args[1]);
 
-	zend_try {
-		retval = ps_call_handler(PSF(open), 2, args TSRMLS_CC);
-	} zend_catch {
-		PS(session_status) = php_session_none;
-		if (retval) {
-			zval_ptr_dtor(&retval);
-		}
-		zend_bailout();
-	} zend_end_try();
-
+	retval = ps_call_handler(PSF(open), 2, args TSRMLS_CC);
 	PS(mod_user_implemented) = 1;
 
 	FINISH;
diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c
index 6768bec867..392f09eda1 100644
--- a/ext/session/mod_user_class.c
+++ b/ext/session/mod_user_class.c
@@ -22,10 +22,6 @@
 #include "php_session.h"
 
 #define PS_SANITY_CHECK						\
-	if (PS(session_status) != php_session_active) { \
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Session is not active"); \
-		RETURN_FALSE; \
-	} \
 	if (PS(default_mod) == NULL) {				\
 		php_error_docref(NULL TSRMLS_CC, E_CORE_ERROR, "Cannot call default session handler"); \
 		RETURN_FALSE;						\
@@ -43,7 +39,7 @@
 PHP_METHOD(SessionHandler, open)
 {
 	char *save_path = NULL, *session_name = NULL;
-	int ret, save_path_len, session_name_len;
+	int save_path_len, session_name_len;
 
 	PS_SANITY_CHECK;
 
@@ -52,14 +48,7 @@ PHP_METHOD(SessionHandler, open)
 	}
 
 	PS(mod_user_is_open) = 1;
-	zend_try {
-		ret =PS(default_mod)->s_open(&PS(mod_data), save_path, session_name TSRMLS_CC);
-	} zend_catch {
-		PS(session_status) = php_session_none;
-		zend_bailout();
-	} zend_end_try();
-
-	RETVAL_BOOL(SUCCESS == ret);
+	RETVAL_BOOL(SUCCESS == PS(default_mod)->s_open(&PS(mod_data), save_path, session_name TSRMLS_CC));
 }
 /* }}} */
 
@@ -67,8 +56,6 @@ 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
@@ -76,15 +63,7 @@ PHP_METHOD(SessionHandler, close)
 	zend_parse_parameters_none();
 	
 	PS(mod_user_is_open) = 0;
-
-	zend_try {
-		ret = PS(default_mod)->s_close(&PS(mod_data) TSRMLS_CC);
-	} zend_catch {
-		PS(session_status) = php_session_none;
-		zend_bailout();
-	} zend_end_try();
-
-	RETVAL_BOOL(SUCCESS == ret);
+	RETVAL_BOOL(SUCCESS == PS(default_mod)->s_close(&PS(mod_data) TSRMLS_CC));
 }
 /* }}} */
 
diff --git a/ext/session/session.c b/ext/session/session.c
index c3ea950908..b4e5a327c1 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -87,19 +87,15 @@ zend_class_entry *php_session_id_iface_entry;
 	}
 
 static void php_session_send_cookie(TSRMLS_D);
-static void php_session_abort(TSRMLS_D);
 
 /* Dispatched by RINIT and by php_session_destroy */
 static inline void php_rinit_session_globals(TSRMLS_D) /* {{{ */
 {
-	/* 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;
 	PS(mod_user_is_open) = 0;
-	PS(define_sid) = 1;
+	/* Do NOT init PS(mod_user_names) here! */
 	PS(http_session_vars) = NULL;
 }
 /* }}} */
@@ -121,9 +117,6 @@ static inline void php_rshutdown_session_globals(TSRMLS_D) /* {{{ */
 		efree(PS(id));
 		PS(id) = 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;
 }
 /* }}} */
 
@@ -475,56 +468,26 @@ PHPAPI int php_session_valid_key(const char *key) /* {{{ */
 }
 /* }}} */
 
-
-static void php_session_gc(TSRMLS_D) /* {{{ */
-{
-	int nrand;
-
-	/* GC must be done before reading session data. */
-	if ((PS(mod_data) || PS(mod_user_implemented)) && PS(gc_probability) > 0) {
-		int nrdels = -1;
-
-		nrand = (int) ((float) PS(gc_divisor) * php_combined_lcg(TSRMLS_C));
-		if (nrand < PS(gc_probability)) {
-			PS(mod)->s_gc(&PS(mod_data), PS(gc_maxlifetime), &nrdels TSRMLS_CC);
-#ifdef SESSION_DEBUG
-			if (nrdels != -1) {
-				php_error_docref(NULL TSRMLS_CC, E_NOTICE, "purged %d expired session objects", nrdels);
-			}
-#endif
-		}
-	}
-} /* }}} */
-
-
 static void php_session_initialize(TSRMLS_D) /* {{{ */
 {
 	char *val = NULL;
 	int vallen;
 
-	PS(session_status) = php_session_active;
-
 	if (!PS(mod)) {
-		PS(session_status) = php_session_disabled;
 		php_error_docref(NULL TSRMLS_CC, E_ERROR, "No storage module chosen - failed to initialize session");
 		return;
 	}
 
 	/* Open session handler first */
 	if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name) TSRMLS_CC) == FAILURE) {
-		php_session_abort(TSRMLS_C);
 		php_error_docref(NULL TSRMLS_CC, E_ERROR, "Failed to initialize storage module: %s (path: %s)", PS(mod)->s_name, PS(save_path));
 		return;
 	}
 
 	/* If there is no ID, use session module to create one */
-	if (!PS(id) || !PS(id)[0]) {
-		if (PS(id)) {
-			efree(PS(id));
-		}
+	if (!PS(id)) {
 		PS(id) = PS(mod)->s_create_sid(&PS(mod_data), NULL TSRMLS_CC);
 		if (!PS(id)) {
-			php_session_abort(TSRMLS_C);
 			php_error_docref(NULL TSRMLS_CC, E_ERROR, "Failed to create session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
 			return;
 		}
@@ -536,22 +499,20 @@ static void php_session_initialize(TSRMLS_D) /* {{{ */
 	/* Set session ID for compatibility for older/3rd party save handlers */
 	if (!PS(use_strict_mode)) {
 		php_session_reset_id(TSRMLS_C);
+		PS(session_status) = php_session_active;
 	}
 
-	/* GC must be done before read */
-	php_session_gc(TSRMLS_C);
-
 	/* 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_WARNING, "Failed to read session data: %s (path: %s)", PS(mod)->s_name, PS(save_path)); */
-		/* return; */
+		/*
+		php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Failed to read session data: %s (path: %s)", PS(mod)->s_name, PS(save_path));
+		*/
 	}
 	/* Set session ID if session read didn't activated session */
-	if (PS(use_strict_mode) && PS(session_status) == php_session_none) {
+	if (PS(use_strict_mode) && PS(session_status) != php_session_active) {
 		php_session_reset_id(TSRMLS_C);
 		PS(session_status) = php_session_active;
 	}
@@ -1119,7 +1080,7 @@ static ps_serializer ps_serializers[MAX_SERIALIZERS + 1] = {
 
 PHPAPI int php_session_register_serializer(const char *name, int (*encode)(PS_SERIALIZER_ENCODE_ARGS), int (*decode)(PS_SERIALIZER_DECODE_ARGS)) /* {{{ */
 {
-	int ret = FAILURE;
+	int ret = -1;
 	int i;
 
 	for (i = 0; i < MAX_SERIALIZERS; i++) {
@@ -1128,7 +1089,7 @@ PHPAPI int php_session_register_serializer(const char *name, int (*encode)(PS_SE
 			ps_serializers[i].encode = encode;
 			ps_serializers[i].decode = decode;
 			ps_serializers[i + 1].name = NULL;
-			ret = SUCCESS;
+			ret = 0;
 			break;
 		}
 	}
@@ -1150,13 +1111,13 @@ static ps_module *ps_modules[MAX_MODULES + 1] = {
 
 PHPAPI int php_session_register_module(ps_module *ptr) /* {{{ */
 {
-	int ret = FAILURE;
+	int ret = -1;
 	int i;
 
 	for (i = 0; i < MAX_MODULES; i++) {
 		if (!ps_modules[i]) {
 			ps_modules[i] = ptr;
-			ret = SUCCESS;
+			ret = 0;
 			break;
 		}
 	}
@@ -1294,13 +1255,11 @@ 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);
 		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 {
@@ -1531,6 +1490,7 @@ PHPAPI void php_session_start(TSRMLS_D) /* {{{ */
 	zval **ppid;
 	zval **data;
 	char *p, *value;
+	int nrand;
 	int lensess;
 
 	if (PS(use_only_cookies)) {
@@ -1650,14 +1610,28 @@ PHPAPI void php_session_start(TSRMLS_D) /* {{{ */
 
 	php_session_initialize(TSRMLS_C);
 	php_session_cache_limiter(TSRMLS_C);
+
+	if ((PS(mod_data) || PS(mod_user_implemented)) && PS(gc_probability) > 0) {
+		int nrdels = -1;
+
+		nrand = (int) ((float) PS(gc_divisor) * php_combined_lcg(TSRMLS_C));
+		if (nrand < PS(gc_probability)) {
+			PS(mod)->s_gc(&PS(mod_data), PS(gc_maxlifetime), &nrdels TSRMLS_CC);
+#ifdef SESSION_DEBUG
+			if (nrdels != -1) {
+				php_error_docref(NULL TSRMLS_CC, E_NOTICE, "purged %d expired session objects", nrdels);
+			}
+#endif
+		}
+	}
 }
 /* }}} */
 
 static void php_session_flush(TSRMLS_D) /* {{{ */
 {
 	if (PS(session_status) == php_session_active) {
-		php_session_save_current_state(TSRMLS_C);
 		PS(session_status) = php_session_none;
+		php_session_save_current_state(TSRMLS_C);
 	}
 }
 /* }}} */
@@ -1665,10 +1639,10 @@ static void php_session_flush(TSRMLS_D) /* {{{ */
 static void php_session_abort(TSRMLS_D) /* {{{ */
 {
 	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) TSRMLS_CC);
 		}
-		PS(session_status) = php_session_none;
 	}
 }
 /* }}} */
@@ -2111,6 +2085,10 @@ static PHP_FUNCTION(session_decode)
 static PHP_FUNCTION(session_start)
 {
 	/* skipping check for non-zero args for performance reasons here ?*/
+	if (PS(id) && !strlen(PS(id))) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot start session with empty session ID");
+		RETURN_FALSE;
+	}
 
 	php_session_start(TSRMLS_C);
 
diff --git a/ext/session/tests/bug32330.phpt b/ext/session/tests/bug32330.phpt
index fe83cc9504..98d442ae5c 100644
--- a/ext/session/tests/bug32330.phpt
+++ b/ext/session/tests/bug32330.phpt
@@ -69,17 +69,17 @@ $_SESSION['E'] = 'F';
 ?>
 --EXPECTF--
 open: path = /tmp, name = sid
-gc: maxlifetime = %d
 read: id = %s
+gc: maxlifetime = %d
 write: id = %s, data = A|s:1:"B";
 close
 open: path = /tmp, name = sid
-gc: maxlifetime = %d
 read: id = %s
+gc: maxlifetime = %d
 destroy: id = %s
 close
 open: path = /tmp, name = sid
-gc: maxlifetime = %d
 read: id = %s
+gc: maxlifetime = %d
 write: id = %s, data = E|s:1:"F";
 close
diff --git a/ext/session/tests/bug55688.phpt b/ext/session/tests/bug55688.phpt
index b073dc3c5c..8db48384af 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(): Session is not active in %s on line %d
+Warning: SessionHandler::gc(): Parent session handler is not open in %s on line %d
diff --git a/ext/session/tests/bug60634.phpt b/ext/session/tests/bug60634.phpt
index b2f5076287..86dcb11526 100644
--- a/ext/session/tests/bug60634.phpt
+++ b/ext/session/tests/bug60634.phpt
@@ -39,17 +39,8 @@ 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 7388ba0123..e41592f18d 100644
--- a/ext/session/tests/bug60634_error_1.phpt
+++ b/ext/session/tests/bug60634_error_1.phpt
@@ -40,20 +40,9 @@ 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
 
 Fatal error: Call to undefined function undefined_function() in %s on line %d
-write: goodbye cruel world
-
-Fatal error: Call to undefined function undefined_function() in %s on line %d
 close: goodbye cruel world
diff --git a/ext/session/tests/bug60634_error_5.phpt b/ext/session/tests/bug60634_error_5.phpt
index 07d7b3a514..8081ab988a 100644
--- a/ext/session/tests/bug60634_error_5.phpt
+++ b/ext/session/tests/bug60634_error_5.phpt
@@ -45,5 +45,3 @@ echo "um, hi\n";
 close: goodbye cruel world
 
 Fatal error: Call to undefined function undefined_function() in %s on line %d
-
-Warning: Unknown: Failed to write session data (user). Please verify that the current setting of session.save_path is correct () in Unknown on line 0
diff --git a/ext/session/tests/bug61728.phpt b/ext/session/tests/bug61728.phpt
index 2780d7b7e2..3f8dbeb58a 100644
--- a/ext/session/tests/bug61728.phpt
+++ b/ext/session/tests/bug61728.phpt
@@ -8,34 +8,32 @@ function output_html($ext) {
     return strlen($ext);
 }
 
-function open ($save_path, $session_name) {
+function open ($save_path, $session_name) { 
     return true;
-}
+} 
 
-function close() {
+function close() { 
     return true;
-}
+} 
 
-function read ($id) {
-    return '';
-}
+function read ($id) { 
+} 
 
-function write ($id, $sess_data) {
+function write ($id, $sess_data) { 
     ob_start("output_html");
     echo "laruence";
     ob_end_flush();
     return true;
-}
+} 
 
-function destroy ($id) {
-    return true;
-}
+function destroy ($id) { 
+} 
 
-function gc ($maxlifetime) {
-    return true;
-}
+function gc ($maxlifetime) { 
+    return true; 
+} 
 
-session_set_save_handler ("open", "close", "read", "write", "destroy", "gc");
+session_set_save_handler ("open", "close", "read", "write", "destroy", "gc"); 
 session_start();
 --EXPECTF--
 8
diff --git a/ext/session/tests/bug67972.phpt b/ext/session/tests/bug67972.phpt
index 92c3044ac5..63ed3a95b8 100644
--- a/ext/session/tests/bug67972.phpt
+++ b/ext/session/tests/bug67972.phpt
@@ -7,5 +7,4 @@ Bug #67972: SessionHandler Invalid memory read create_sid()
 
 (new SessionHandler)->create_sid();
 --EXPECTF--
-Warning: SessionHandler::create_sid(): Session is not active in %s on line %d
-
+Fatal error: SessionHandler::create_sid(): Cannot call default session handler in %s on line %d
diff --git a/ext/session/tests/bug68063.phpt b/ext/session/tests/bug68063.phpt
index ec3a70d156..d3da470d06 100644
--- a/ext/session/tests/bug68063.phpt
+++ b/ext/session/tests/bug68063.phpt
@@ -3,22 +3,18 @@ Bug #68063 (Empty session IDs do still start sessions)
 --SKIPIF--
 <?php include('skipif.inc'); ?>
 --INI--
-session.use_strict_mode=0
-session.hash_function=1
-session.hash_bits_per_character=4
 --FILE--
 <?php
-// Empty session ID may happen by browser bugs
-
 // Could also be set with a cookie like "PHPSESSID=; path=/"
 session_id('');
 
-// Start the session with empty string should result in new session ID
+// Will still start the session and return true
 var_dump(session_start());
 
-// Returns newly created session ID
+// Returns an empty string
 var_dump(session_id());
 ?>
 --EXPECTF--
-bool(true)
-string(40) "%s"
+Warning: session_start(): Cannot start session with empty session ID in %s on line %d
+bool(false)
+string(0) ""
diff --git a/ext/session/tests/bug69111.phpt b/ext/session/tests/bug69111.phpt
deleted file mode 100644
index f5def0ed35..0000000000
--- a/ext/session/tests/bug69111.phpt
+++ /dev/null
@@ -1,36 +0,0 @@
---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/session_set_save_handler_variation4.phpt b/ext/session/tests/session_set_save_handler_variation4.phpt
index 6923828409..3485f2373a 100644
--- a/ext/session/tests/session_set_save_handler_variation4.phpt
+++ b/ext/session/tests/session_set_save_handler_variation4.phpt
@@ -51,8 +51,8 @@ ob_end_flush();
 *** Testing session_set_save_handler() : variation ***
 
 Open [%s,PHPSESSID]
-GC [0]
 Read [%s,%s]
+GC [0]
 array(3) {
   ["Blah"]=>
   string(12) "Hello World!"
@@ -65,9 +65,15 @@ Write [%s,%s,Blah|s:12:"Hello World!";Foo|b:0;Guff|i:1234567890;]
 Close [%s,PHPSESSID]
 NULL
 Open [%s,PHPSESSID]
-GC [0]
 Read [%s,%s]
-array(0) {
+GC [0]
+array(3) {
+  ["Blah"]=>
+  string(12) "Hello World!"
+  ["Foo"]=>
+  bool(false)
+  ["Guff"]=>
+  int(1234567890)
 }
 Destroy [%s,%s]
 
diff --git a/ext/session/tests/sessionhandler_open_001.phpt b/ext/session/tests/sessionhandler_open_001.phpt
index e6e913a6a5..6ade9e00a5 100644
--- a/ext/session/tests/sessionhandler_open_001.phpt
+++ b/ext/session/tests/sessionhandler_open_001.phpt
@@ -16,11 +16,4 @@ 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!