fix(worker): initialize _request in worker mode with auto_globals

[xavierleune]

Hi,

This PR fixes #1931, it handles $_REQUEST in worker mode correctly when auto_globals_jit is enabled (default configuration for PHP).
Some concerns were raised in the comments of the issue regarding performance. This implementation should make sure that request is created only if used.

However if a previous execution plan already used _REQUEST, all subsequent requests will create it. So the concern is "kindof" mitigated.

Let me know if you have any suggestion to improve this.

[dunglas]

LGTM

[Copilot]

Pull request overview

This PR fixes a bug in worker mode where the $_REQUEST superglobal was not being properly reinitialized between requests when PHP's auto_globals_jit is enabled (the default configuration). The fix ensures $_REQUEST contains only the current request's data, not stale data from previous requests.

Changes:

• Modified frankenphp_reset_super_globals() to properly handle JIT auto globals by re-initializing them if they exist in the symbol table, or re-arming them for future use
• Added comprehensive test coverage including basic and conditional access scenarios to verify $_REQUEST correctness across multiple requests

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
frankenphp.c	Updated superglobal reset logic to handle $_REQUEST initialization in worker mode with JIT auto globals
frankenphp_test.go	Added test functions to verify $_REQUEST behavior in both module and worker modes
testdata/request-superglobal.php	Test script that outputs $_REQUEST, $_GET, and $_POST to verify correct merging
testdata/request-superglobal-conditional.php	Test script for conditional $_REQUEST access to verify re-arm mechanism
testdata/request-superglobal-conditional-include.php	Included file that validates $_REQUEST contains only current request data

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[xavierleune]

Is the failing test the kind of situation where we should try turning it off and on again ? :D

[AlliBalliBaba]

Good idea to only reset $_REQUEST if it's present in the symbol table.

Do we need to reset $_GLOBALS as well though? Couldn't that have other side effects?

[AlliBalliBaba]

Suggested change

	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)

[AlliBalliBaba]

Suggested change

	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)

[AlliBalliBaba]

Suggested change

	// $_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)
	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'")
	assert.Contains(t, body, "VAL_CHECK:MATCH", "BUG: $_REQUEST contains stale data from previous request! Body: "+body)

[AlliBalliBaba]

The failing test was testing super globals reset,.so likely related to this PR.