Reset serialization state before request input processing

Zend/tests/arginfo_zpp_mismatch.inc

@@ -10,6 +10,7 @@ function skipFunction($function): bool {
      || $function === 'exit'
      || $function === 'die'
      || $function === 'zend_trigger_bailout'
+     || $function === 'zend_test_serialization_bailout'
         /* intentionally violate invariants */
      || $function === 'zend_create_unterminated_string'
      || $function === 'zend_test_array_return'

ext/standard/basic_functions.c

@@ -390,10 +390,6 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */
 {
 	memset(BG(strtok_table), 0, 256);
 
-	BG(serialize_lock) = 0;
-	memset(&BG(serialize), 0, sizeof(BG(serialize)));
-	memset(&BG(unserialize), 0, sizeof(BG(unserialize)));
-
 	BG(strtok_string) = NULL;
 	BG(strtok_last) = NULL;
 	BG(ctype_string) = NULL;

ext/standard/php_var.h

@@ -26,6 +26,9 @@ PHPAPI zend_result php_var_export_ex(zval *struc, int level, smart_str *buf);
 
 PHPAPI void php_debug_zval_dump(zval *struc, int level);
 
+/* Reset before request input processing, never during serialization. */
+PHPAPI void php_var_serialization_reset(void);
+
 typedef struct php_serialize_data *php_serialize_data_t;
 typedef struct php_unserialize_data *php_unserialize_data_t;
 

ext/standard/var.c

@@ -1406,6 +1406,14 @@ PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t
 }
 /* }}} */
 
+PHPAPI void php_var_serialization_reset(void)
+{
+	/* A bailout can abandon contexts that the request heap will reclaim. */
+	BG(serialize_lock) = 0;
+	memset(&BG(serialize), 0, sizeof(BG(serialize)));
+	memset(&BG(unserialize), 0, sizeof(BG(unserialize)));
+}
+
 PHPAPI php_serialize_data_t php_var_serialize_init(void) {
 	struct php_serialize_data *d;
 	/* fprintf(stderr, "SERIALIZE_INIT      == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */

ext/zend_test/test.c

@@ -21,6 +21,7 @@
 #include "php.h"
 #include "php_ini.h"
 #include "ext/standard/info.h"
+#include "ext/standard/php_var.h"
 #include "php_test.h"
 #include "observer.h"
 #include "fiber.h"
@@ -200,6 +201,38 @@ static ZEND_FUNCTION(zend_leak_bytes)
 	emalloc(leakbytes);
 }
 
+static ZEND_FUNCTION(zend_test_serialization_bailout)
+{
+	bool unserialize, locked;
+
+	ZEND_PARSE_PARAMETERS_START(2, 2)
+		Z_PARAM_BOOL(unserialize)
+		Z_PARAM_BOOL(locked)
+	ZEND_PARSE_PARAMETERS_END();
+
+	if (unserialize) {
+		(void) php_var_unserialize_init();
+	} else {
+		(void) php_var_serialize_init();
+	}
+	if (locked) {
+		BG(serialize_lock)++;
+	}
+	zend_bailout();
+}
+
+static ZEND_FUNCTION(zend_test_serialization_state)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+
+	array_init(return_value);
+	add_assoc_long(return_value, "lock", BG(serialize_lock));
+	add_assoc_long(return_value, "serialize_level", BG(serialize).level);
+	add_assoc_bool(return_value, "serialize_data", BG(serialize).data != NULL);
+	add_assoc_long(return_value, "unserialize_level", BG(unserialize).level);
+	add_assoc_bool(return_value, "unserialize_data", BG(unserialize).data != NULL);
+}
+
 /* Reallocate a block of old_size bytes to new_size bytes and return its block
  * size, along with the block size of a fresh allocation of new_size bytes. */
 static ZEND_FUNCTION(zend_test_erealloc_block_size)

ext/zend_test/test.stub.php

@@ -301,6 +301,10 @@ function zend_delref(mixed $variable): void {}
 
     function zend_test_erealloc_block_size(int $old_size, int $new_size): array {}
 
+    function zend_test_serialization_bailout(bool $unserialize, bool $locked): never {}
+
+    function zend_test_serialization_state(): array {}
+
 	function zend_bool(bool $param): bool {}
 	function zend_bool_or_null(bool|null $param): bool|null {}
 	function zend_bool_slow_zpp(bool $param): bool {}

main/main.c

@@ -1854,6 +1854,8 @@ zend_result php_request_startup(void)
 {
 	zend_result retval = SUCCESS;
 
+	/* Input processing can use serialization before module RINIT. */
+	php_var_serialization_reset();
 	zend_interned_strings_activate();
 
 #ifdef HAVE_DTRACE

sapi/cli/tests/serialization_context_request_reset.phpt

@@ -0,0 +1,172 @@
+--TEST--
+Serialization contexts abandoned by a bailout are reset before processing the next request
+--EXTENSIONS--
+session
+zend_test
+--INI--
+allow_url_fopen=1
+--ENV--
+PHP_CLI_SERVER_WORKERS=
+--SKIPIF--
+<?php
+include "skipif.inc";
+if (!function_exists("proc_open")) {
+    die("skip proc_open is required");
+}
+?>
+--FILE--
+<?php
+include "php_cli_server.inc";
+
+$sessionDir = __DIR__ . "/serialization_context_request_reset-sessions-" . getmypid();
+mkdir($sessionDir);
+register_shutdown_function(function () use ($sessionDir) {
+    $path = $sessionDir . "/sess_serialization-reset";
+    if (file_exists($path)) {
+        unlink($path);
+    }
+    rmdir($sessionDir);
+});
+$args = [
+    "-d", "session.upload_progress.enabled=1",
+    "-d", "session.upload_progress.cleanup=0",
+    "-d", "session.upload_progress.freq=0",
+    "-d", "session.upload_progress.min_freq=0",
+    "-d", "session.use_strict_mode=0",
+    "-d", "session.save_handler=files",
+    "-d", "session.save_path=$sessionDir",
+];
+foreach (["session", "zend_test"] as $extension) {
+    $path = ini_get("extension_dir") . DIRECTORY_SEPARATOR . $extension . "." . PHP_SHLIB_SUFFIX;
+    if (file_exists($path)) {
+        $args[] = "-d";
+        $args[] = "extension=$path";
+    }
+}
+$server = php_cli_server_start(<<<'PHP'
+session_id("serialization-reset");
+session_start();
+echo getmypid(), "\n";
+if (isset($_GET["bailout"])) {
+    $_SESSION = ["value" => ["nested" => 42]];
+    session_write_close();
+    echo "bailout\n";
+    zend_test_serialization_bailout((bool) $_GET["unserialize"], (bool) $_GET["locked"]);
+}
+var_dump(zend_test_serialization_state());
+var_dump($_SESSION["value"]);
+var_dump($_SESSION["upload_progress_test"]["done"]);
+var_dump($_FILES["file"]["error"]);
+session_destroy();
+PHP, cmd_args: $args);
+
+$body = "--boundary\r\n"
+    . "Content-Disposition: form-data; name=\"PHP_SESSION_UPLOAD_PROGRESS\"\r\n\r\n"
+    . "test\r\n"
+    . "--boundary\r\n"
+    . "Content-Disposition: form-data; name=\"file\"; filename=\"test.txt\"\r\n"
+    . "Content-Type: text/plain\r\n\r\n"
+    . "contents\r\n"
+    . "--boundary--\r\n";
+
+foreach ([0, 1] as $unserialize) {
+    foreach ([0, 1] as $locked) {
+        [$pid, $response] = explode("\n", file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS
+            . "/?bailout=1&unserialize=$unserialize&locked=$locked"), 2);
+        echo $response;
+        $context = stream_context_create(["http" => [
+            "method" => "POST",
+            "header" => "Content-Type: multipart/form-data; boundary=boundary\r\n"
+                . "Cookie: PHPSESSID=serialization-reset\r\n",
+            "content" => $body,
+        ]]);
+        $response = file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, false, $context);
+        if ($response === false) {
+            echo file_get_contents($server->outputFile);
+            break 2;
+        }
+        [$nextPid, $response] = explode("\n", $response, 2);
+        if ($nextPid !== $pid) {
+            throw new RuntimeException("Requests must use the same worker");
+        }
+        echo $response;
+    }
+}
+?>
+--EXPECT--
+bailout
+array(5) {
+  ["lock"]=>
+  int(0)
+  ["serialize_level"]=>
+  int(0)
+  ["serialize_data"]=>
+  bool(false)
+  ["unserialize_level"]=>
+  int(0)
+  ["unserialize_data"]=>
+  bool(false)
+}
+array(1) {
+  ["nested"]=>
+  int(42)
+}
+bool(true)
+int(0)
+bailout
+array(5) {
+  ["lock"]=>
+  int(0)
+  ["serialize_level"]=>
+  int(0)
+  ["serialize_data"]=>
+  bool(false)
+  ["unserialize_level"]=>
+  int(0)
+  ["unserialize_data"]=>
+  bool(false)
+}
+array(1) {
+  ["nested"]=>
+  int(42)
+}
+bool(true)
+int(0)
+bailout
+array(5) {
+  ["lock"]=>
+  int(0)
+  ["serialize_level"]=>
+  int(0)
+  ["serialize_data"]=>
+  bool(false)
+  ["unserialize_level"]=>
+  int(0)
+  ["unserialize_data"]=>
+  bool(false)
+}
+array(1) {
+  ["nested"]=>
+  int(42)
+}
+bool(true)
+int(0)
+bailout
+array(5) {
+  ["lock"]=>
+  int(0)
+  ["serialize_level"]=>
+  int(0)
+  ["serialize_data"]=>
+  bool(false)
+  ["unserialize_level"]=>
+  int(0)
+  ["unserialize_data"]=>
+  bool(false)
+}
+array(1) {
+  ["nested"]=>
+  int(42)
+}
+bool(true)
+int(0)