Commit 86b49211577 for php.net
commit 86b49211577ffe146950b561cbe7e9aacfdcf674
Author: Gina Peter Banyard <girgias@php.net>
Date: Mon Feb 16 11:44:15 2026 +0000
ext/session: only return false when could not encode session at all (#21181)
* ext/session: only return false when could not encode session at all
This also fixes bug 71162
diff --git a/ext/session/session.c b/ext/session/session.c
index d9a4e2b5a4d..5a05700608c 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -507,24 +507,24 @@ static void php_session_save_current_state(bool write)
IF_SESSION_VARS() {
zval *handler_function = &PS(mod_user_names).ps_write;
if (PS(mod_data) || PS(mod_user_implemented)) {
- zend_string *val;
-
- val = php_session_encode();
- if (val) {
- if (PS(lazy_write) && PS(session_vars)
- && PS(mod)->s_update_timestamp
- && PS(mod)->s_update_timestamp != php_session_update_timestamp
- && zend_string_equals(val, PS(session_vars))
- ) {
- ret = PS(mod)->s_update_timestamp(&PS(mod_data), PS(id), val, PS(gc_maxlifetime));
- handler_function = &PS(mod_user_names).ps_update_timestamp;
- } else {
- ret = PS(mod)->s_write(&PS(mod_data), PS(id), val, PS(gc_maxlifetime));
- }
- zend_string_release_ex(val, false);
+ zend_string *val = php_session_encode();
+ /* Not being able to encode the session data means there is some kind of issue that prevents a write
+ * (e.g. a key containing the '|' character with the default serialization) */
+ if (UNEXPECTED(val == NULL)) {
+ return;
+ }
+
+ if (PS(lazy_write) && PS(session_vars)
+ && PS(mod)->s_update_timestamp
+ && PS(mod)->s_update_timestamp != php_session_update_timestamp
+ && zend_string_equals(val, PS(session_vars))
+ ) {
+ ret = PS(mod)->s_update_timestamp(&PS(mod_data), PS(id), val, PS(gc_maxlifetime));
+ handler_function = &PS(mod_user_names).ps_update_timestamp;
} else {
- ret = PS(mod)->s_write(&PS(mod_data), PS(id), ZSTR_EMPTY_ALLOC(), PS(gc_maxlifetime));
+ ret = PS(mod)->s_write(&PS(mod_data), PS(id), val, PS(gc_maxlifetime));
}
+ zend_string_release_ex(val, false);
}
if ((ret == FAILURE) && !EG(exception)) {
@@ -929,7 +929,7 @@ PS_SERIALIZER_ENCODE_FUNC(php_serialize)
php_var_serialize(&buf, Z_REFVAL(PS(http_session_vars)), &var_hash);
PHP_VAR_SERIALIZE_DESTROY(var_hash);
}
- return buf.s;
+ return smart_str_extract(&buf);
}
PS_SERIALIZER_DECODE_FUNC(php_serialize)
@@ -980,11 +980,9 @@ PS_SERIALIZER_ENCODE_FUNC(php_binary)
smart_str_append(&buf, key);
php_var_serialize(&buf, struc, &var_hash);
);
-
- smart_str_0(&buf);
PHP_VAR_SERIALIZE_DESTROY(var_hash);
- return buf.s;
+ return smart_str_extract(&buf);
}
PS_SERIALIZER_DECODE_FUNC(php_binary)
@@ -1038,7 +1036,6 @@ PS_SERIALIZER_ENCODE_FUNC(php)
PS_ENCODE_LOOP(
smart_str_append(&buf, key);
if (memchr(ZSTR_VAL(key), PS_DELIMITER, ZSTR_LEN(key))) {
- PHP_VAR_SERIALIZE_DESTROY(var_hash);
smart_str_free(&buf);
fail = true;
php_error_docref(NULL, E_WARNING, "Failed to write session data. Data contains invalid key \"%s\"", ZSTR_VAL(key));
@@ -1047,15 +1044,15 @@ PS_SERIALIZER_ENCODE_FUNC(php)
smart_str_appendc(&buf, PS_DELIMITER);
php_var_serialize(&buf, struc, &var_hash);
);
+ PHP_VAR_SERIALIZE_DESTROY(var_hash);
if (fail) {
return NULL;
}
- smart_str_0(&buf);
+ zend_string *encoded = smart_str_extract(&buf);
- PHP_VAR_SERIALIZE_DESTROY(var_hash);
- return buf.s;
+ return encoded;
}
PS_SERIALIZER_DECODE_FUNC(php)
@@ -2280,7 +2277,6 @@ PHP_FUNCTION(session_id)
PHP_FUNCTION(session_regenerate_id)
{
bool del_ses = false;
- zend_string *data;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|b", &del_ses) == FAILURE) {
RETURN_THROWS();
@@ -2307,14 +2303,17 @@ PHP_FUNCTION(session_regenerate_id)
RETURN_FALSE;
}
} else {
- zend_result ret;
- data = php_session_encode();
- if (data) {
- ret = PS(mod)->s_write(&PS(mod_data), PS(id), data, PS(gc_maxlifetime));
- zend_string_release_ex(data, false);
- } else {
- ret = PS(mod)->s_write(&PS(mod_data), PS(id), ZSTR_EMPTY_ALLOC(), PS(gc_maxlifetime));
+ zend_string *old_session_data = php_session_encode();
+ /* If we have no data we must destroy the related session ID */
+ if (UNEXPECTED(old_session_data == NULL)) {
+ PS(mod)->s_close(&PS(mod_data));
+ PS(session_status) = php_session_none;
+ RETURN_FALSE;
}
+
+ zend_result ret = PS(mod)->s_write(&PS(mod_data), PS(id), old_session_data, PS(gc_maxlifetime));
+ zend_string_release_ex(old_session_data, false);
+
if (ret == FAILURE) {
PS(mod)->s_close(&PS(mod_data));
PS(session_status) = php_session_none;
@@ -2368,6 +2367,7 @@ PHP_FUNCTION(session_regenerate_id)
// TODO warn that ID cannot be verified? else { }
}
/* Read is required to make new session data at this point. */
+ zend_string *data = NULL;
if (PS(mod)->s_read(&PS(mod_data), PS(id), &data, PS(gc_maxlifetime)) == FAILURE) {
PS(mod)->s_close(&PS(mod_data));
PS(session_status) = php_session_none;
diff --git a/ext/session/tests/session_encode_error2.phpt b/ext/session/tests/session_encode_error2.phpt
index e112d693539..42f5c4dd688 100644
--- a/ext/session/tests/session_encode_error2.phpt
+++ b/ext/session/tests/session_encode_error2.phpt
@@ -51,28 +51,28 @@
bool(true)
Warning: session_encode(): Skipping numeric key 0 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
-- Iteration 2 --
bool(true)
Warning: session_encode(): Skipping numeric key 1 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
-- Iteration 3 --
bool(true)
Warning: session_encode(): Skipping numeric key 12345 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
-- Iteration 4 --
bool(true)
Warning: session_encode(): Skipping numeric key -2345 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
-- Iteration 5 --
diff --git a/ext/session/tests/session_encode_partial_data.phpt b/ext/session/tests/session_encode_partial_data.phpt
new file mode 100644
index 00000000000..9236dbcb111
--- /dev/null
+++ b/ext/session/tests/session_encode_partial_data.phpt
@@ -0,0 +1,30 @@
+--TEST--
+session_encode(): partial session data
+--INI--
+serialize_precision=100
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+session_start();
+
+$_SESSION['data1'] = 'hello';
+$_SESSION['data2'] = 'PHP';
+$_SESSION['partial|data'] = 'key with pipe';
+$_SESSION['data3'] = 'world';
+
+var_dump(session_encode());
+
+var_dump(session_destroy());
+
+ob_end_flush();
+?>
+--EXPECTF--
+Warning: session_encode(): Failed to write session data. Data contains invalid key "partial|data" in %s on line %d
+bool(false)
+bool(true)
diff --git a/ext/session/tests/session_encode_variation1.phpt b/ext/session/tests/session_encode_variation1.phpt
index 1b5be5e42dc..4a948eba69c 100644
--- a/ext/session/tests/session_encode_variation1.phpt
+++ b/ext/session/tests/session_encode_variation1.phpt
@@ -30,11 +30,11 @@
Warning: session_encode(): Cannot encode non-existent session in %s on line %d
bool(false)
bool(true)
-bool(false)
+string(0) ""
bool(true)
-bool(false)
+string(0) ""
bool(true)
-bool(false)
+string(0) ""
bool(true)
Warning: session_encode(): Cannot encode non-existent session in %s on line %d
diff --git a/ext/session/tests/session_encode_variation2.phpt b/ext/session/tests/session_encode_variation2.phpt
index 8803c308069..3984ec71100 100644
--- a/ext/session/tests/session_encode_variation2.phpt
+++ b/ext/session/tests/session_encode_variation2.phpt
@@ -22,7 +22,7 @@
?>
--EXPECTF--
*** Testing session_encode() : variation ***
-bool(false)
+string(0) ""
bool(true)
Warning: session_encode(): Cannot encode non-existent session in %s on line %d
diff --git a/ext/session/tests/session_encode_variation6.phpt b/ext/session/tests/session_encode_variation6.phpt
index 301dbd61679..2b3ab0e78a0 100644
--- a/ext/session/tests/session_encode_variation6.phpt
+++ b/ext/session/tests/session_encode_variation6.phpt
@@ -32,16 +32,16 @@
bool(true)
Warning: session_encode(): Skipping numeric key 0 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
bool(true)
Warning: session_encode(): Skipping numeric key 1234567890 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
bool(true)
Warning: session_encode(): Skipping numeric key -1234567890 in %s on line %d
-bool(false)
+string(0) ""
bool(true)
Done
diff --git a/ext/session/tests/user_session_module/bug71162.phpt b/ext/session/tests/user_session_module/bug71162.phpt
index 673e6ac2ecb..54679973dc3 100644
--- a/ext/session/tests/user_session_module/bug71162.phpt
+++ b/ext/session/tests/user_session_module/bug71162.phpt
@@ -4,8 +4,6 @@
session
--INI--
session.use_strict_mode=0
---XFAIL--
-Current session module is designed to write empty session always.
--FILE--
<?php
class MySessionHandler extends SessionHandler implements SessionUpdateTimestampHandlerInterface
@@ -71,7 +69,7 @@ public function updateTimestamp($sessid, $sessdata): bool {
--EXPECT--
string(40) "da39a3ee5e6b4b0d3255bfef95601890afd80709"
bool(true)
-write
+updateTimestamp
string(40) "da39a3ee5e6b4b0d3255bfef95601890afd80709"
bool(true)
updateTimestamp
diff --git a/ext/session/tests/user_session_module/session_set_save_handler_variation5.phpt b/ext/session/tests/user_session_module/session_set_save_handler_variation5.phpt
index 9c584eec597..a3e21c35e49 100644
--- a/ext/session/tests/user_session_module/session_set_save_handler_variation5.phpt
+++ b/ext/session/tests/user_session_module/session_set_save_handler_variation5.phpt
@@ -83,7 +83,7 @@ function noisy_gc($maxlifetime) {
GC [0]
1 deleted
bool(true)
-Write [%s,PHPT-%d,]
+Update [%s,PHPT-%d]
Close [%s,PHPSESSID]
bool(true)
string(%d) "PHPT-%d"