fix: unset worker mode error handler while parsing the request

frankenphp.c

@@ -685,6 +685,11 @@ void get_full_env(zval *track_vars_array) {
 static int frankenphp_worker_request_startup() {
   int retval = SUCCESS;
 
+  /* Use the default error handler while parsing the worker request */
+  zval orig_user_error_handler;
+  ZVAL_COPY_VALUE(&orig_user_error_handler, &EG(user_error_handler));
+  ZVAL_UNDEF(&EG(user_error_handler));
+
   frankenphp_update_request_context();
 
   zend_try {
@@ -747,6 +752,9 @@ static int frankenphp_worker_request_startup() {
   zend_catch { retval = FAILURE; }
   zend_end_try();
 
+  /* Restore the original user error handler */
+  ZVAL_COPY_VALUE(&EG(user_error_handler), &orig_user_error_handler);
+
   SG(sapi_started) = 1;
 
   return retval;

testdata/error-handler.php

@@ -0,0 +1,22 @@
+<?php
+
+require_once __DIR__ . '/_executor.php';
+
+set_error_handler(function ($severity, $message, $file, $line) {
+    echo "an error has occurred";
+});
+
+$requests = 0;
+
+return function () use (&$requests) {
+    ++$requests;
+    echo "requests:$requests\n";
+
+    // get_error_handler is not available in PHP <= 8.4
+    if (function_exists('get_error_handler')) {
+        $handler = get_error_handler();
+        echo $handler ? "handler is defined" : "handler is not defined";
+    } else {
+        echo "handler is defined";
+    }
+};

testdata/request-parse-errors.php

@@ -0,0 +1,23 @@
+<?php
+
+require_once __DIR__ . '/_executor.php';
+
+// Throwing handler mimicking symfony/error-handler: before the fix, parse
+// warnings raised between requests escaped as an uncaught ErrorException and
+// killed the worker script (https://github.com/php/frankenphp/issues/2631).
+set_error_handler(function ($severity, $message, $file, $line) {
+    throw new \ErrorException($message, 0, $severity, $file, $line);
+});
+
+$requests = 0;
+
+return function () use (&$requests) {
+    $lastError = error_get_last();
+    if ($lastError && $lastError['type'] === E_WARNING) {
+        http_response_code(400);
+        echo $lastError['message'];
+    }
+
+    ++$requests;
+    echo "requests:{$requests}\n";
+};

worker_test.go

@@ -46,6 +46,46 @@ func TestWorker(t *testing.T) {
 	}, &testOptions{workerScript: "worker.php", nbWorkers: 1, nbParallelRequests: 1})
 }
 
+func TestWorkerSurvivesRequestParseErrors(t *testing.T) {
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		// max_input_vars exceeded while the worker script has a throwing error
+		// handler installed (https://github.com/php/frankenphp/issues/2631)
+		params := make([]string, 20)
+		for i := range params {
+			params[i] = fmt.Sprintf("p%d=%d", i, i)
+		}
+		body, resp := testGet("http://example.com/request-parse-errors.php?"+strings.Join(params, "&"), handler, t)
+		assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+		assert.Contains(t, body, "requests:1")
+		assert.Contains(t, body, "Input variables exceeded")
+
+		// post_max_size exceeded: the same worker must have survived the
+		// previous parse failure (the counter proves no restart happened)
+		req := httptest.NewRequest("POST", "http://example.com/request-parse-errors.php", strings.NewReader(strings.Repeat("a=1&", 600)))
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		body2, resp2 := testRequest(req, handler, t)
+		assert.Equal(t, http.StatusBadRequest, resp2.StatusCode)
+		assert.Contains(t, body2, "requests:2")
+		assert.Contains(t, body2, "exceeds the limit")
+
+		// clean request: recorded errors must be reset between requests
+		body3, resp3 := testGet("http://example.com/request-parse-errors.php", handler, t)
+		assert.Equal(t, http.StatusOK, resp3.StatusCode)
+		assert.Contains(t, body3, "requests:3")
+	}, &testOptions{workerScript: "request-parse-errors.php", nbWorkers: 1, nbParallelRequests: 1, phpIni: map[string]string{"max_input_vars": "10", "post_max_size": "1K", "display_errors": "0"}})
+}
+
+func TestErrorHandlerPersistsAcrossRequests(t *testing.T) {
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		for i := 0; i < 5; i++ {
+			body, resp := testGet("http://example.com/error-handler.php", handler, t)
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+			assert.Contains(t, body, "handler is defined")
+			assert.Contains(t, body, fmt.Sprintf("requests:%d", i+1))
+		}
+	}, &testOptions{workerScript: "error-handler.php", nbWorkers: 1, nbParallelRequests: 1})
+}
+
 func TestWorkerDie(t *testing.T) {
 	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) {
 		req := httptest.NewRequest("GET", "http://example.com/die.php", nil)