From 2e165e1a065a7e24a9ab2a8b62b30655d83f87ac Mon Sep 17 00:00:00 2001 From: sunwoolee Date: Sun, 25 Oct 2026 21:39:03 +0900 Subject: [PATCH] Fix GH-24493: preserve session records and release aborted upload buffers --- ext/session/session.c | 11 ++ .../tests/gh24493_upload_progress_memory.phpt | 130 +++++++++++++++ ...493_upload_progress_session_roundtrip.phpt | 151 ++++++++++++++++++ ...c1867_incomplete_class_reserved_props.phpt | 19 ++- 4 files changed, 301 insertions(+), 10 deletions(-) create mode 100644 ext/session/tests/gh24493_upload_progress_memory.phpt create mode 100644 ext/session/tests/gh24493_upload_progress_session_roundtrip.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 7a57652f55d1..2e4840bccb7b 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -266,6 +266,17 @@ static zend_string *php_session_encode(void) /* {{{ */ static ZEND_COLD void php_session_cancel_decode(void) { + if (PS(in_rfc1867_update)) { + /* The upload progress decoder is deliberately more restrictive than + * session_start(). Leave the stored record for the application to read. */ + php_session_abort(); + /* An aborted update will not use the lazy-write comparison buffer. */ + if (PS(session_vars)) { + zend_string_release_ex(PS(session_vars), 0); + PS(session_vars) = NULL; + } + return; + } php_session_destroy(); php_session_track_init(); php_error_docref(NULL, E_WARNING, "Failed to decode session object. Session has been destroyed"); diff --git a/ext/session/tests/gh24493_upload_progress_memory.phpt b/ext/session/tests/gh24493_upload_progress_memory.phpt new file mode 100644 index 000000000000..45edbdecb543 --- /dev/null +++ b/ext/session/tests/gh24493_upload_progress_memory.phpt @@ -0,0 +1,130 @@ +--TEST-- +GH-24493: aborted upload progress updates release the lazy-write buffer +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- +__PHP_Incomplete_Class_Name = 'application value'; + /* The restricted decoder rejects the object before reading the payload. */ + $_SESSION = ['object' => $object, 'payload' => str_repeat('A', 5 * 1024 * 1024)]; + session_write_close(); + echo "SEEDED\n"; + return; +} +$ok = ($_SESSION['object']->__PHP_Incomplete_Class_Name ?? null) === 'application value' + && ($_SESSION['payload'] ?? null) === str_repeat('A', 5 * 1024 * 1024); +session_write_close(); +echo $ok ? "PRESERVED\n" : "LOST\n"; +CODE); +$boundary = 'sessionmemoryboundary'; +$body = "--$boundary\r\nContent-Disposition: form-data; name=\"PHP_SESSION_UPLOAD_PROGRESS\"\r\n\r\nkey\r\n" + . "--$boundary\r\nContent-Disposition: form-data; name=\"file\"; filename=\"file.txt\"\r\n\r\nabc\r\n" + . "--$boundary--\r\n"; +$ini = ['session.save_path' => $dir, 'session.save_handler' => 'files', + 'session.gc_probability' => '0', 'session.use_strict_mode' => '0', + 'session.use_cookies' => '1', 'session.use_only_cookies' => '1', + 'session.cache_limiter' => '', 'session.name' => 'PHPSESSID', + 'session.lazy_write' => '1', 'session.upload_progress.enabled' => '1', + 'session.upload_progress.name' => 'PHP_SESSION_UPLOAD_PROGRESS', + 'session.upload_progress.freq' => '1', 'session.upload_progress.min_freq' => '0', + 'file_uploads' => '1', 'display_errors' => '1', 'html_errors' => '0']; +$extensionDir = ini_get('extension_dir'); +foreach (['session.' . PHP_SHLIB_SUFFIX, 'php_session.' . PHP_SHLIB_SUFFIX] as $module) { + if ($extensionDir !== false && file_exists("$extensionDir/$module")) { + $ini['extension_dir'] = $extensionDir; + $ini['extension'] = $module; + break; + } +} +foreach (['php', 'php_binary', 'php_serialize'] as $serializer) { + foreach ([0, 1] as $cleanup) { + $sid = 'memory' . $cleanup; + @unlink("$dir/sess_$sid"); + $ok = true; + foreach (['seed', 'upload', 'read'] as $action) { + $cmd = [getenv('TEST_PHP_CGI_EXECUTABLE'), '-n', '-q']; + $settings = $ini + ['session.serialize_handler' => $serializer, + 'session.upload_progress.cleanup' => (string) $cleanup, + 'memory_limit' => $action === 'upload' ? '8M' : '64M']; + foreach ($settings as $name => $value) { + $cmd[] = '-d'; + $cmd[] = "$name=$value"; + } + $cmd[] = '-f'; + $cmd[] = $worker; + $env = getenv(); + $env['REDIRECT_STATUS'] = '1'; + $env['REQUEST_METHOD'] = $action === 'upload' ? 'POST' : 'GET'; + $env['SCRIPT_FILENAME'] = $worker; + $env['PATH_TRANSLATED'] = $worker; + $env['QUERY_STRING'] = ''; + $env['HTTP_COOKIE'] = "PHPSESSID=$sid"; + $env['CONTENT_TYPE'] = "multipart/form-data; boundary=$boundary"; + $env['CONTENT_LENGTH'] = $action === 'upload' ? (string) strlen($body) : '0'; + $env['TEST_ACTION'] = $action; + $proc = proc_open($cmd, [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes, $dir, $env); + if ($action === 'upload') { + fwrite($pipes[0], $body); + } + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $error = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + $exit = proc_close($proc); + $expected = match ($action) { + 'seed' => 'SEEDED', 'upload' => 'UPLOAD:0:abc:clean', 'read' => 'PRESERVED' + }; + $ok = $ok && $exit === 0 && $error === '' && str_contains($output, $expected) + && !str_contains($output, 'Warning:') && !str_contains($output, 'Fatal error:'); + if ($action === 'seed') { + $recordHash = hash_file('sha256', "$dir/sess_$sid"); + } elseif ($action === 'upload') { + $ok = $ok && file_exists("$dir/sess_$sid") + && hash_file('sha256', "$dir/sess_$sid") === $recordHash; + } + } + printf("%s cleanup=%d %s\n", $serializer, $cleanup, $ok ? 'OK' : 'FAIL'); + } +} +?> +--CLEAN-- + +--EXPECT-- +php cleanup=0 OK +php cleanup=1 OK +php_binary cleanup=0 OK +php_binary cleanup=1 OK +php_serialize cleanup=0 OK +php_serialize cleanup=1 OK diff --git a/ext/session/tests/gh24493_upload_progress_session_roundtrip.phpt b/ext/session/tests/gh24493_upload_progress_session_roundtrip.phpt new file mode 100644 index 000000000000..4cbbffaf096d --- /dev/null +++ b/ext/session/tests/gh24493_upload_progress_session_roundtrip.phpt @@ -0,0 +1,151 @@ +--TEST-- +GH-24493: upload progress preserves application-generated session records across serializers +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- +{$property} = 'BUSINESS_LITERAL'; + $_SESSION = ['object' => $object, 'alias' => $object, 'sentinel' => 'KEEP_LITERAL']; + session_write_close(); + echo "SEEDED\n"; + return; +} +$object = $_SESSION['object'] ?? null; +$ok = $object instanceof stdClass + && ($object->{$property} ?? null) === 'BUSINESS_LITERAL' + && ($_SESSION['sentinel'] ?? null) === 'KEEP_LITERAL' + && $object === ($_SESSION['alias'] ?? null); +session_write_close(); +echo $ok ? "PRESERVED\n" : "LOST\n"; +CODE); +$properties = ['normal', '__PHP_Incomplete_Class_Name', + '__PHP_Incomplete_Class_Serialized', '__PHP_Incomplete_Enum_Case']; +$boundary = 'gh24493boundary'; +$body = "--$boundary\r\nContent-Disposition: form-data; name=\"PHP_SESSION_UPLOAD_PROGRESS\"\r\n\r\nkey\r\n" + . "--$boundary\r\nContent-Disposition: form-data; name=\"file\"; filename=\"normal.txt\"\r\n\r\nabc\r\n" + . "--$boundary--\r\n"; +$ini = ['session.save_path' => $dir, 'session.save_handler' => 'files', + 'session.use_strict_mode' => '0', 'session.use_cookies' => '1', + 'session.use_only_cookies' => '1', 'session.cache_limiter' => '', + 'session.name' => 'PHPSESSID', 'session.upload_progress.enabled' => '1', + 'session.upload_progress.name' => 'PHP_SESSION_UPLOAD_PROGRESS', + 'session.upload_progress.min_freq' => '0', 'session.upload_progress.freq' => '1', + 'file_uploads' => '1', 'display_errors' => '1', 'html_errors' => '0']; +$extensionDir = ini_get('extension_dir'); +foreach (['session.' . PHP_SHLIB_SUFFIX, 'php_session.' . PHP_SHLIB_SUFFIX] as $module) { + if ($extensionDir !== false && file_exists("$extensionDir/$module")) { + $ini['extension_dir'] = $extensionDir; + $ini['extension'] = $module; + break; + } +} +foreach (['php', 'php_binary', 'php_serialize'] as $serializer) { + foreach ([0, 1] as $cleanup) { + foreach ($properties as $index => $property) { + $sid = 'roundtrip' . $cleanup . $index; + @unlink("$dir/sess_$sid"); + $cmd = [getenv('TEST_PHP_CGI_EXECUTABLE'), '-n', '-q']; + $settings = $ini + ['session.serialize_handler' => $serializer, + 'session.upload_progress.cleanup' => (string) $cleanup]; + foreach ($settings as $name => $value) { + $cmd[] = '-d'; + $cmd[] = "$name=$value"; + } + $cmd[] = '-f'; + $cmd[] = $worker; + $ok = true; + foreach (['seed', 'upload', 'read'] as $action) { + $env = getenv(); + $env['REDIRECT_STATUS'] = '1'; + $env['REQUEST_METHOD'] = $action === 'upload' ? 'POST' : 'GET'; + $env['SCRIPT_FILENAME'] = $worker; + $env['PATH_TRANSLATED'] = $worker; + $env['QUERY_STRING'] = ''; + $env['HTTP_COOKIE'] = "PHPSESSID=$sid"; + $env['CONTENT_TYPE'] = "multipart/form-data; boundary=$boundary"; + $env['CONTENT_LENGTH'] = $action === 'upload' ? (string) strlen($body) : '0'; + $env['TEST_PROPERTY'] = $property; + $env['TEST_ACTION'] = $action; + $proc = proc_open($cmd, [0 => ['pipe','r'], 1 => ['pipe','w'], 2 => ['pipe','w']], $pipes, $dir, $env); + if ($action === 'upload') { + fwrite($pipes[0], $body); + } + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $error = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + $exit = proc_close($proc); + $expected = match ($action) { + 'seed' => 'SEEDED', 'upload' => 'UPLOAD:0:clean', 'read' => 'PRESERVED' + }; + $ok = $ok && $exit === 0 && $error === '' && str_contains($output, $expected) + && !str_contains($output, 'Warning:'); + if ($action === 'seed') { + $record = file_get_contents("$dir/sess_$sid"); + } elseif ($action === 'upload' && $property !== 'normal') { + /* A rejected progress update must not rewrite even a partial record. */ + $ok = $ok && file_exists("$dir/sess_$sid") + && file_get_contents("$dir/sess_$sid") === $record; + } + } + printf("%s cleanup=%d property=%d %s\n", $serializer, $cleanup, $index, $ok ? 'OK' : 'FAIL'); + } + } +} +?> +--CLEAN-- + +--EXPECT-- +php cleanup=0 property=0 OK +php cleanup=0 property=1 OK +php cleanup=0 property=2 OK +php cleanup=0 property=3 OK +php cleanup=1 property=0 OK +php cleanup=1 property=1 OK +php cleanup=1 property=2 OK +php cleanup=1 property=3 OK +php_binary cleanup=0 property=0 OK +php_binary cleanup=0 property=1 OK +php_binary cleanup=0 property=2 OK +php_binary cleanup=0 property=3 OK +php_binary cleanup=1 property=0 OK +php_binary cleanup=1 property=1 OK +php_binary cleanup=1 property=2 OK +php_binary cleanup=1 property=3 OK +php_serialize cleanup=0 property=0 OK +php_serialize cleanup=0 property=1 OK +php_serialize cleanup=0 property=2 OK +php_serialize cleanup=0 property=3 OK +php_serialize cleanup=1 property=0 OK +php_serialize cleanup=1 property=1 OK +php_serialize cleanup=1 property=2 OK +php_serialize cleanup=1 property=3 OK diff --git a/ext/session/tests/rfc1867_incomplete_class_reserved_props.phpt b/ext/session/tests/rfc1867_incomplete_class_reserved_props.phpt index 954e46b7116b..064817546953 100644 --- a/ext/session/tests/rfc1867_incomplete_class_reserved_props.phpt +++ b/ext/session/tests/rfc1867_incomplete_class_reserved_props.phpt @@ -1,5 +1,5 @@ --TEST-- -session rfc1867: the upload progress update rejects session data whose objects carry the incomplete class bookkeeping property names +session rfc1867: the upload progress update preserves records it cannot decode without user code --EXTENSIONS-- session --SKIPIF-- @@ -23,7 +23,8 @@ file_put_contents($worker, ' 'x|O:8:"stdClass":1:{s:1:"a";i:1;}', 'Class_Name' => 'x|O:8:"stdClass":1:{s:27:"__PHP_Incomplete_Class_Name";s:3:"foo";}', @@ -109,9 +110,7 @@ foreach ($cases as $label => $record) { fclose($pipes[2]); proc_close($proc); - /* A refused decode must leave exactly one warning: the handler stops there - * instead of writing the progress into the empty session it was left with, - * which would add a second warning about the save path. */ + /* A progress-only decode failure must leave the stored session intact. */ $kept = file_exists($file) && str_starts_with(file_get_contents($file), $record); printf("%-16s warnings: %d, decode refused: %-5s record kept: %s\n", $label, substr_count($out, 'Warning:'), @@ -131,9 +130,9 @@ foreach (glob("$dir/sess_*") as $f) { ?> --EXPECT-- control warnings: 0, decode refused: false record kept: true -Class_Name warnings: 1, decode refused: true record kept: false -Class_Serialized warnings: 1, decode refused: true record kept: false -Enum_Case warnings: 1, decode refused: true record kept: false -nested warnings: 1, decode refused: true record kept: false +Class_Name warnings: 0, decode refused: false record kept: true +Class_Serialized warnings: 0, decode refused: false record kept: true +Enum_Case warnings: 0, decode refused: false record kept: true +nested warnings: 0, decode refused: false record kept: true mangled warnings: 0, decode refused: false record kept: true -corrupt warnings: 1, decode refused: true record kept: false +corrupt warnings: 0, decode refused: false record kept: true