Fix Bug #73461

This patch disables any invalid save handler calls.

6 files changed, 96 insertions, 11 deletions

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index 0cdbaf96f9..0a54204cc9 100644
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -75,7 +75,15 @@ PS_OPEN_FUNC(user)
 	zval args[2];
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	if (Z_ISUNDEF(PSF(open))) {
+		PS(in_save_handler) = 0;
 		php_error_docref(NULL, E_WARNING,
 			"user session functions not defined");
 
@@ -88,6 +96,7 @@ PS_OPEN_FUNC(user)
 	zend_try {
 		ps_call_handler(&PSF(open), 2, args, &retval);
 	} zend_catch {
+		PS(in_save_handler) = 0;
 		PS(session_status) = php_session_none;
 		if (!Z_ISUNDEF(retval)) {
 			zval_ptr_dtor(&retval);
@@ -97,6 +106,7 @@ PS_OPEN_FUNC(user)
 
 	PS(mod_user_implemented) = 1;
 
+	PS(in_save_handler) = 0;
 	FINISH;
 }
 
@@ -105,8 +115,16 @@ PS_CLOSE_FUNC(user)
 	zend_bool bailout = 0;
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	if (!PS(mod_user_implemented)) {
 		/* already closed */
+		PS(in_save_handler) = 0;
 		return SUCCESS;
 	}
 
@@ -119,12 +137,14 @@ PS_CLOSE_FUNC(user)
 	PS(mod_user_implemented) = 0;
 
 	if (bailout) {
+		PS(in_save_handler) = 0;
 		if (!Z_ISUNDEF(retval)) {
 			zval_ptr_dtor(&retval);
 		}
 		zend_bailout();
 	}
 
+	PS(in_save_handler) = 0;
 	FINISH;
 }
 
@@ -133,6 +153,13 @@ PS_READ_FUNC(user)
 	zval args[1];
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	ZVAL_STR_COPY(&args[0], key);
 
 	ps_call_handler(&PSF(read), 1, args, &retval);
@@ -145,6 +172,7 @@ PS_READ_FUNC(user)
 		zval_ptr_dtor(&retval);
 	}
 
+	PS(in_save_handler) = 0;
 	return ret;
 }
 
@@ -153,11 +181,19 @@ PS_WRITE_FUNC(user)
 	zval args[2];
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	ZVAL_STR_COPY(&args[0], key);
 	ZVAL_STR_COPY(&args[1], val);
 
 	ps_call_handler(&PSF(write), 2, args, &retval);
 
+	PS(in_save_handler) = 0;
 	FINISH;
 }
 
@@ -166,10 +202,18 @@ PS_DESTROY_FUNC(user)
 	zval args[1];
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	ZVAL_STR_COPY(&args[0], key);
 
 	ps_call_handler(&PSF(destroy), 1, args, &retval);
 
+	PS(in_save_handler) = 0;
 	FINISH;
 }
 
@@ -178,24 +222,41 @@ PS_GC_FUNC(user)
 	zval args[1];
 	zval retval;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	ZVAL_LONG(&args[0], maxlifetime);
 
 	ps_call_handler(&PSF(gc), 1, args, &retval);
 
 	if (Z_TYPE(retval) == IS_LONG) {
 		convert_to_long(&retval);
+		PS(in_save_handler) = 0;
 		return Z_LVAL(retval);
 	}
 	/* This is for older API compatibility */
 	if (Z_TYPE(retval) == IS_TRUE) {
+		PS(in_save_handler) = 0;
 		return 1;
 	}
+	PS(in_save_handler) = 0;
 	/* Anything else is some kind of error */
 	return -1; // Error
 }
 
 PS_CREATE_SID_FUNC(user)
 {
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	/* maintain backwards compatibility */
 	if (!Z_ISUNDEF(PSF(create_sid))) {
 		zend_string *id = NULL;
@@ -209,24 +270,35 @@ PS_CREATE_SID_FUNC(user)
 			}
 			zval_ptr_dtor(&retval);
 		} else {
+			PS(in_save_handler) = 0;
 			zend_throw_error(NULL, "No session id returned by function");
 			return NULL;
 		}
 
 		if (!id) {
+			PS(in_save_handler) = 0;
 			zend_throw_error(NULL, "Session id must be a string");
 			return NULL;
 		}
 
+		PS(in_save_handler) = 0;
 		return id;
 	}
 
+	PS(in_save_handler) = 0;
 	/* function as defined by PS_MOD */
 	return php_session_create_id(mod_data);
 }
 
 PS_VALIDATE_SID_FUNC(user)
 {
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	/* maintain backwards compatibility */
 	if (!Z_ISUNDEF(PSF(validate_sid))) {
 		zval args[1];
@@ -236,9 +308,11 @@ PS_VALIDATE_SID_FUNC(user)
 
 		ps_call_handler(&PSF(validate_sid), 1, args, &retval);
 
+		PS(in_save_handler) = 0;
 		FINISH;
 	}
 
+	PS(in_save_handler) = 0;
 	/* dummy function defined by PS_MOD */
 	return php_session_validate_sid(mod_data, key);
 }
@@ -248,6 +322,13 @@ PS_UPDATE_TIMESTAMP_FUNC(user)
 	zval args[2];
 	STDVARS;
 
+	if (PS(in_save_handler)) {
+		PS(in_save_handler) = 0;
+		php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+		return FAILURE;
+	}
+	PS(in_save_handler) = 1;
+
 	ZVAL_STR_COPY(&args[0], key);
 	ZVAL_STR_COPY(&args[1], val);
 
@@ -258,6 +339,7 @@ PS_UPDATE_TIMESTAMP_FUNC(user)
 		ps_call_handler(&PSF(write), 2, args, &retval);
 	}
 
+	PS(in_save_handler) = 0;
 	FINISH;
 }
 
diff --git a/ext/session/php_session.h b/ext/session/php_session.h
index da5e48515a..775527c2f6 100644
--- a/ext/session/php_session.h
+++ b/ext/session/php_session.h
@@ -205,6 +205,7 @@ typedef struct _php_ps_globals {
 	zend_bool use_strict_mode; /* whether or not PHP accepts unknown session ids */
 	zend_bool lazy_write; /* omit session write when it is possible */
 	zend_string *session_vars; /* serialized original session data */
+	zend_bool in_save_handler; /* state that if session is in save handler or not */
 } php_ps_globals;
 
 typedef php_ps_globals zend_ps_globals;
diff --git a/ext/session/session.c b/ext/session/session.c
index 5484390c7e..bbc531aa73 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -106,6 +106,7 @@ static inline void php_rinit_session_globals(void) /* {{{ */
 	/* 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(in_save_handler) = 0;
 	PS(mod_data) = NULL;
 	PS(mod_user_is_open) = 0;
 	PS(define_sid) = 1;
@@ -2035,7 +2036,7 @@ static PHP_FUNCTION(session_create_id)
 		}
 	}
 
-	if (PS(session_status) == php_session_active) {
+	if (!PS(in_save_handler) && PS(session_status) == php_session_active) {
 		int limit = 3;
 		while (limit--) {
 			new_id = PS(mod)->s_create_sid(&PS(mod_data));
diff --git a/ext/session/tests/bug60634.phpt b/ext/session/tests/bug60634.phpt
index b2f5076287..c21c6360a6 100644
--- a/ext/session/tests/bug60634.phpt
+++ b/ext/session/tests/bug60634.phpt
@@ -40,16 +40,16 @@ 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().
-*/
+ * This test raises error in Unknown function because 2nd close write handler is
+ * called at request shutdown and session module detects recursive call like
+ * multiple save handler calls.
+ */
 
 ?>
 --EXPECTF--
 write: goodbye cruel world
-write: goodbye cruel world
-close: goodbye cruel world
 
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0
+
+Warning: Unknown: Failed to write session data using user defined save handler. (session.save_path: ) in Unknown on line 0
+close: goodbye cruel world
diff --git a/ext/session/tests/bug60634_error_3.phpt b/ext/session/tests/bug60634_error_3.phpt
index b7840b04f9..b38899b8c1 100644
--- a/ext/session/tests/bug60634_error_3.phpt
+++ b/ext/session/tests/bug60634_error_3.phpt
@@ -48,4 +48,5 @@ Stack trace:
 #0 [internal function]: write(%s, '')
 #1 {main}
   thrown in %s on line %d
-close: goodbye cruel world
+
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0
diff --git a/ext/session/tests/bug60634_error_4.phpt b/ext/session/tests/bug60634_error_4.phpt
index 7970b35c7a..3ac6247974 100644
--- a/ext/session/tests/bug60634_error_4.phpt
+++ b/ext/session/tests/bug60634_error_4.phpt
@@ -48,5 +48,5 @@ Stack trace:
 #0 [internal function]: write('%s', '')
 #1 {main}
   thrown in %s on line %d
-close: goodbye cruel world
 
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0