From ae22ddc7552335ff0e7c4be8ec0b765430294055 Mon Sep 17 00:00:00 2001 From: Xavier Leune Date: Thu, 22 Jan 2026 15:32:18 +0100 Subject: [PATCH 1/5] fix(worker): initialize _request in worker mode with auto_globals --- frankenphp.c | 15 +++-- frankenphp_test.go | 55 +++++++++++++++++++ ...equest-superglobal-conditional-include.php | 20 +++++++ testdata/request-superglobal-conditional.php | 16 ++++++ testdata/request-superglobal.php | 14 +++++ 5 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 testdata/request-superglobal-conditional-include.php create mode 100644 testdata/request-superglobal-conditional.php create mode 100644 testdata/request-superglobal.php diff --git a/frankenphp.c b/frankenphp.c index fd487edb8e..c89c445a5f 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -126,13 +126,18 @@ static void frankenphp_reset_super_globals() { if (auto_global->name == _env) { /* skip $_ENV */ } else if (auto_global->name == _server) { - /* always reimport $_SERVER */ + /* always reimport $_SERVER */ auto_global->armed = auto_global->auto_global_callback(auto_global->name); } else if (auto_global->jit) { - /* globals with jit are: $_SERVER, $_ENV, $_REQUEST, $GLOBALS, - * jit will only trigger on script parsing and therefore behaves - * differently in worker mode. We will skip all jit globals - */ + /* JIT globals ($_REQUEST, $GLOBALS) need special handling: + * - If in symbol_table: re-initialize with current request data + * - If not: re-arm for potential future use during include */ + if (zend_hash_exists(&EG(symbol_table), auto_global->name)) { + auto_global->armed = + auto_global->auto_global_callback(auto_global->name); + } else { + auto_global->armed = true; + } } else if (auto_global->auto_global_callback) { /* $_GET, $_POST, $_COOKIE, $_FILES are reimported here */ auto_global->armed = auto_global->auto_global_callback(auto_global->name); diff --git a/frankenphp_test.go b/frankenphp_test.go index 8c6f3c90da..d76e4a1f51 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -306,6 +306,61 @@ func testPostSuperGlobals(t *testing.T, opts *testOptions) { }, opts) } +func TestRequestSuperGlobal_module(t *testing.T) { testRequestSuperGlobal(t, nil) } +func TestRequestSuperGlobal_worker(t *testing.T) { + phpIni := make(map[string]string) + phpIni["opcache.jit"] = "tracing" + testRequestSuperGlobal(t, &testOptions{workerScript: "request-superglobal.php", phpIni: phpIni}) +} +func testRequestSuperGlobal(t *testing.T, opts *testOptions) { + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { + // Test with both GET and POST parameters + // $_REQUEST should contain merged data from both + formData := url.Values{"post_key": {fmt.Sprintf("post_value_%d", i)}} + req := httptest.NewRequest("POST", fmt.Sprintf("http://example.com/request-superglobal.php?get_key=get_value_%d", i), strings.NewReader(formData.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + body, _ := testRequest(req, handler, t) + + // Verify $_REQUEST contains both GET and POST data for the current request + assert.Contains(t, body, fmt.Sprintf("'get_key' => 'get_value_%d'", i)) + assert.Contains(t, body, fmt.Sprintf("'post_key' => 'post_value_%d'", i)) + }, opts) +} + +func TestRequestSuperGlobalConditional_worker(t *testing.T) { + // This test verifies that $_REQUEST works correctly when accessed conditionally + // in worker mode. The first request does NOT access $_REQUEST, but subsequent + // requests do. This tests the "re-arm" mechanism for JIT auto globals. + // + // The bug scenario: + // - Request 1 (i=1): includes file, $_REQUEST initialized with val=1 + // - Request 3 (i=3): includes file from cache, $_REQUEST should have val=3 + // If the bug exists, $_REQUEST would still have val=1 from request 1. + phpIni := make(map[string]string) + phpIni["opcache.jit"] = "tracing" + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { + if i%2 == 0 { + // Even requests: don't use $_REQUEST + req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/request-superglobal-conditional.php?val=%d", i), nil) + body, _ := testRequest(req, handler, t) + assert.Contains(t, body, "SKIPPED") + assert.Contains(t, body, fmt.Sprintf("'val' => '%d'", i)) + } else { + // Odd requests: use $_REQUEST + req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/request-superglobal-conditional.php?use_request=1&val=%d", i), nil) + body, _ := testRequest(req, handler, t) + assert.Contains(t, body, "REQUEST:") + // $_REQUEST should have ONLY current request's data (2 keys: use_request and val) + assert.Contains(t, body, "REQUEST_COUNT:2") + // Verify current request data is present + assert.Contains(t, body, fmt.Sprintf("'val' => '%d'", i)) + assert.Contains(t, body, "'use_request' => '1'") + // Critical: verify $_REQUEST has the CURRENT request's val, not stale data + assert.Contains(t, body, "VAL_CHECK:MATCH", "BUG: $_REQUEST contains stale data from previous request! Body: "+body) + } + }, &testOptions{workerScript: "request-superglobal-conditional.php", phpIni: phpIni}) +} + func TestCookies_module(t *testing.T) { testCookies(t, nil) } func TestCookies_worker(t *testing.T) { testCookies(t, &testOptions{workerScript: "cookies.php"}) } func testCookies(t *testing.T, opts *testOptions) { diff --git a/testdata/request-superglobal-conditional-include.php b/testdata/request-superglobal-conditional-include.php new file mode 100644 index 0000000000..f41bb98adf --- /dev/null +++ b/testdata/request-superglobal-conditional-include.php @@ -0,0 +1,20 @@ + Date: Tue, 27 Jan 2026 11:58:41 +0100 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Signed-off-by: Xavier Leune --- frankenphp_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/frankenphp_test.go b/frankenphp_test.go index d76e4a1f51..6edc0302ef 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -341,21 +341,16 @@ func TestRequestSuperGlobalConditional_worker(t *testing.T) { runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { if i%2 == 0 { // Even requests: don't use $_REQUEST - req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/request-superglobal-conditional.php?val=%d", i), nil) - body, _ := testRequest(req, handler, t) + body, _:= testGet(fmt.Sprintf("http://example.com/request-superglobal-conditional.php?val=%d", i), handler, t) assert.Contains(t, body, "SKIPPED") assert.Contains(t, body, fmt.Sprintf("'val' => '%d'", i)) } else { // Odd requests: use $_REQUEST - req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/request-superglobal-conditional.php?use_request=1&val=%d", i), nil) - body, _ := testRequest(req, handler, t) + body, _ := testGet(fmt.Sprintf("http://example.com/request-superglobal-conditional.php?use_request=1&val=%d", i), handler, t) assert.Contains(t, body, "REQUEST:") - // $_REQUEST should have ONLY current request's data (2 keys: use_request and val) - assert.Contains(t, body, "REQUEST_COUNT:2") - // Verify current request data is present - assert.Contains(t, body, fmt.Sprintf("'val' => '%d'", i)) + assert.Contains(t, body, "REQUEST_COUNT:2", "$_REQUEST should have ONLY current request's data (2 keys: use_request and val)") + assert.Contains(t, body, fmt.Sprintf("'val' => '%d'", i), "request data is not present") assert.Contains(t, body, "'use_request' => '1'") - // Critical: verify $_REQUEST has the CURRENT request's val, not stale data assert.Contains(t, body, "VAL_CHECK:MATCH", "BUG: $_REQUEST contains stale data from previous request! Body: "+body) } }, &testOptions{workerScript: "request-superglobal-conditional.php", phpIni: phpIni}) From 3c81d8254ded6172a9b87380e7b0d95806996ed7 Mon Sep 17 00:00:00 2001 From: Xavier Leune Date: Tue, 27 Jan 2026 11:59:48 +0100 Subject: [PATCH 3/5] do not touch as its handled by app, no need to re-arm request if not known --- frankenphp.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index c89c445a5f..ef673473fc 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -130,14 +130,14 @@ static void frankenphp_reset_super_globals() { auto_global->armed = auto_global->auto_global_callback(auto_global->name); } else if (auto_global->jit) { /* JIT globals ($_REQUEST, $GLOBALS) need special handling: + * - $GLOBALS will always be handled by the application, we skip it + * For $_REQUEST: * - If in symbol_table: re-initialize with current request data - * - If not: re-arm for potential future use during include */ - if (zend_hash_exists(&EG(symbol_table), auto_global->name)) { - auto_global->armed = - auto_global->auto_global_callback(auto_global->name); - } else { - auto_global->armed = true; - } + * - If not: do nothing, it may be armed by jit later */ + if (auto_global->name == ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_REQUEST) && zend_hash_exists(&EG(symbol_table), auto_global->name)) { + auto_global->armed = + auto_global->auto_global_callback(auto_global->name); + } } else if (auto_global->auto_global_callback) { /* $_GET, $_POST, $_COOKIE, $_FILES are reimported here */ auto_global->armed = auto_global->auto_global_callback(auto_global->name); From 99a4f6342fbca4c06cc709752f2a7f146d6c2d41 Mon Sep 17 00:00:00 2001 From: Xavier Leune Date: Tue, 27 Jan 2026 12:04:39 +0100 Subject: [PATCH 4/5] do not touch as its handled by app, no need to re-arm request if not known --- frankenphp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index ef673473fc..70c35ae8de 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -134,10 +134,11 @@ static void frankenphp_reset_super_globals() { * For $_REQUEST: * - If in symbol_table: re-initialize with current request data * - If not: do nothing, it may be armed by jit later */ - if (auto_global->name == ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_REQUEST) && zend_hash_exists(&EG(symbol_table), auto_global->name)) { - auto_global->armed = - auto_global->auto_global_callback(auto_global->name); - } + if (auto_global->name == ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_REQUEST) && + zend_hash_exists(&EG(symbol_table), auto_global->name)) { + auto_global->armed = + auto_global->auto_global_callback(auto_global->name); + } } else if (auto_global->auto_global_callback) { /* $_GET, $_POST, $_COOKIE, $_FILES are reimported here */ auto_global->armed = auto_global->auto_global_callback(auto_global->name); From ee6781937ad873d6fbfd7b2217359f7ad1d137fa Mon Sep 17 00:00:00 2001 From: Xavier Leune Date: Tue, 27 Jan 2026 13:20:22 +0100 Subject: [PATCH 5/5] disable jit but make sure autoglobals are on --- frankenphp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frankenphp_test.go b/frankenphp_test.go index 6edc0302ef..937853f676 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -309,7 +309,7 @@ func testPostSuperGlobals(t *testing.T, opts *testOptions) { func TestRequestSuperGlobal_module(t *testing.T) { testRequestSuperGlobal(t, nil) } func TestRequestSuperGlobal_worker(t *testing.T) { phpIni := make(map[string]string) - phpIni["opcache.jit"] = "tracing" + phpIni["auto_globals_jit"] = "1" testRequestSuperGlobal(t, &testOptions{workerScript: "request-superglobal.php", phpIni: phpIni}) } func testRequestSuperGlobal(t *testing.T, opts *testOptions) { @@ -337,7 +337,7 @@ func TestRequestSuperGlobalConditional_worker(t *testing.T) { // - Request 3 (i=3): includes file from cache, $_REQUEST should have val=3 // If the bug exists, $_REQUEST would still have val=1 from request 1. phpIni := make(map[string]string) - phpIni["opcache.jit"] = "tracing" + phpIni["auto_globals_jit"] = "1" runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { if i%2 == 0 { // Even requests: don't use $_REQUEST