diff --git a/ext/session/session.c b/ext/session/session.c index d9a4e2b5a4d2b..5a05700608cc2 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 e112d69353933..42f5c4dd6888b 100644 --- a/ext/session/tests/session_encode_error2.phpt +++ b/ext/session/tests/session_encode_error2.phpt @@ -51,28 +51,28 @@ ob_end_flush(); 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 0000000000000..9236dbcb11117 --- /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-- + +--FILE-- + +--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 1b5be5e42dcb6..4a948eba69c0d 100644 --- a/ext/session/tests/session_encode_variation1.phpt +++ b/ext/session/tests/session_encode_variation1.phpt @@ -30,11 +30,11 @@ ob_end_flush(); 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 8803c3080696b..3984ec7110081 100644 --- a/ext/session/tests/session_encode_variation2.phpt +++ b/ext/session/tests/session_encode_variation2.phpt @@ -22,7 +22,7 @@ ob_end_flush(); ?> --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 301dbd61679ac..2b3ab0e78a095 100644 --- a/ext/session/tests/session_encode_variation6.phpt +++ b/ext/session/tests/session_encode_variation6.phpt @@ -32,16 +32,16 @@ ob_end_flush(); 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 673e6ac2ecb19..54679973dc31d 100644 --- a/ext/session/tests/user_session_module/bug71162.phpt +++ b/ext/session/tests/user_session_module/bug71162.phpt @@ -4,8 +4,6 @@ updateTimestamp never called when session data is empty session --INI-- session.use_strict_mode=0 ---XFAIL-- -Current session module is designed to write empty session always. --FILE--