[php-src] PHP-8.6: ext/session: Close save handler when session encoding fails

Author: Ilia Alshanetsky (iliaal) Date: 2026-10-12T06:43:28-04:00 Commit: https://github.com/php/php-src/commit/ae27423c33678ccb5b415a776d0311aaa6744532 Raw diff: https://github.com/php/php-src/commit/ae27423c33678ccb5b415a776d0311aaa6744532.diff ext/session: Close save handler when session encoding fails Since 86b49211577, php_session_save_current_state() returns without calling the save handler's close callback when the serializer returns NULL, e.g. for a $_SESSION key containing '|'. session_write_close() still returns true, but the files handler keeps its LOCK_EX on the session file until request shutdown, blocking concurrent requests for the same session, and a user handler extending SessionHandler has its close() run at shutdown after the session is gone. Close the handler on that return as the rest of the function and session_regenerate_id() do; with an exception pending the engine already refuses to call a user close() callback. Closes GH-24022 Changed paths: A ext/session/tests/session_write_close_encode_failure.phpt M NEWS M ext/session/session.c Diff: diff --git a/NEWS b/NEWS index 4b109061430e..61631670a40e 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,10 @@ PHP NEWS . Fixed GH-24166 (Double-free in Phar::webPhar() in CGI without PATH_INFO). (RigelYoung, Jakub Zelenka) +- Session: + . Fixed session_write_close() keeping the save handler open when the + session data cannot be encoded. (Ilia Alshanetsky) + - Standard: . Fixed chown() and lchown() failing to resolve user names in ZTS builds when getpwnam_r() needs a larger buffer. (Ilia Alshanetsky) diff --git a/ext/session/session.c b/ext/session/session.c index a37a6d12c2c2..0ee8c94cf33e 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -526,6 +526,7 @@ static void php_session_save_current_state(bool write) /* 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)) { + PS(mod)->s_close(&PS(mod_data)); return; } diff --git a/ext/session/tests/session_write_close_encode_failure.phpt b/ext/session/tests/session_write_close_encode_failure.phpt new file mode 100644 index 000000000000..1c562f401df2 --- /dev/null +++ b/ext/session/tests/session_write_close_encode_failure.phpt @@ -0,0 +1,76 @@ +--TEST-- +session_write_close() closes the save handler when encoding the session data fails +--EXTENSIONS-- +session +--SKIPIF-- +<?php include('skipif.inc'); ?> +--INI-- +session.use_cookies=0 +session.cache_limiter= +session.serialize_handler=php +session.save_handler=files +--FILE-- +<?php +ob_start(); +$dir = __DIR__ . '/session_write_close_encode_failure'; +@mkdir($dir); +session_save_path($dir); +session_start(); +$file = $dir . '/sess_' . session_id(); +$_SESSION['bad|key'] = 'value'; +var_dump(session_write_close()); + +$fp = fopen($file, 'r'); +var_dump(flock($fp, LOCK_EX | LOCK_NB)); +fclose($fp); + +session_start(); +$file = $dir . '/sess_' . session_id(); +$_SESSION['closure'] = function () {}; +$_SESSION['bad|key'] = 'value'; +try { + session_write_close(); +} catch (Exception $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} + +$fp = fopen($file, 'r'); +var_dump(flock($fp, LOCK_EX | LOCK_NB)); +fclose($fp); + +class Handler extends SessionHandler +{ + public function close(): bool + { + echo "close\n"; + return parent::close(); + } +} + +session_set_save_handler(new Handler()); +session_start(); +$_SESSION['bad|key'] = 'value'; +var_dump(session_write_close()); +echo "done\n"; +?> +--CLEAN-- +<?php +$dir = __DIR__ . '/session_write_close_encode_failure'; +foreach (glob($dir . '/sess_*') as $file) { + unlink($file); +} +@rmdir($dir); +?> +--EXPECTF-- +Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d +bool(true) +bool(true) + +Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d +Exception: Serialization of 'Closure' is not allowed +bool(true) + +Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d +close +bool(true) +done