fix: sync \ after \ is updated in SiteURIFactory

[rahul05ranjan]

Description

\SiteURIFactory\ updates \\ (and \['QUERY_STRING']) when it detects the route path, but \\ was left stale. Since PHP populates \\ only once at the start of the request, \getVar()\ (which reads \) returned outdated values, breaking \->withRequest()\ for GET parameters.

This PR adds \Superglobals::syncRequest(), which rebuilds \\ from \, \, and \\ according to the
equest_order\ (or \�ariables_order) ini setting, and calls it from \SiteURIFactory\ after \setGetArray()\ in both \parseRequestURI()\ and \parseQueryString().

This is a one-time synchronization at a single, well-defined point in the request lifecycle (as discussed in the issue), not permanent synchronization — matching standard PHP semantics where \\ is populated once.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

Fixes: #9872

Testing:

• Added \SuperglobalsTest::testSyncRequestRebuildsRequestFromGetPostCookie\ and \ estSyncRequestReflectsGetChanges.
• Extended \SiteURIFactoryDetectRoutePathTest::testQueryStringWithQueryString\ to assert \\ is synchronized (the exact reproduction from the issue).

[carson-codeigniter4]

Hi there, @rahul05ranjan! 👋

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

• PRs must be sent to the appropriate branch
• All git commits must be GPG-signed
• Must follow our style guide
• Be commented in the PHP source file
• Be documented in the user guide
• Be unit tested
• Pass all checks in GitHub Actions

Important

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work
on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

[neznaika0]

Hi. See #10205
We deliberately did not add synchronization. You could add a comment about this.

[rahul05ranjan]

Thanks for the pointer, @neznaika0 — I see now that #10205 took this same syncRequest() approach and was closed as "not the desired approach." I'll rework this PR accordingly.

Based on the discussion in #9872, the preferred direction is to avoid mutating $_REQUEST entirely and instead have getVar() return a merged view of $_GET, $_POST, and $_COOKIE (respecting request_order), or change withRequest() to stop relying on getVar(). That also moves us toward eventually deprecating getVar().

I'll drop the Superglobals::syncRequest() method and the SiteURIFactory changes, and rework the fix around getVar()/withRequest() instead. I'll update the PR shortly.

[rahul05ranjan]

Reworked as discussed. The syncRequest() approach is gone.

getVar() now returns a merged view of $_GET, $_POST, and $_COOKIE (respecting request_order/variables_order) instead of reading the stale $_REQUEST, so $_REQUEST is never mutated. This also moves us toward eventually deprecating getVar().

Changes:

• Superglobals::getRequestData() returns the merged data (no mutation).
• RequestTrait::fetchFromArray() extracts the shared filtering/index logic.
• getVar() uses getRequestData() + fetchFromArray().
• Reverted the SiteURIFactory syncRequest() calls.
• Updated tests accordingly.

@neznaika0 @paulbalandan — does this match the direction you had in mind from #9872?

[rahul05ranjan]

Update: the rework is complete and all commits are now GPG-signed.

Summary of the final state:

• getVar() reads a merged view of $_GET/$_POST/$_COOKIE (respecting request_order) instead of the stale $_REQUEST, so $_REQUEST is never mutated.
• Superglobals::getRequestData() returns the merged data; RequestTrait::fetchFromArray() extracts the shared filtering logic.
• Reverted the SiteURIFactory syncRequest() calls.
• Fixed the static-analysis issues (protected method, no short ternary) and updated the tests.

The Carson checks (signed-commits, no-merge-commits, pr-title-linter) are all passing. The remaining GitHub Actions workflows are showing action_required and need a maintainer to approve them to run.

@neznaika0 @paulbalandan — whenever you have a moment, could you take a look? Thanks!

[neznaika0]

I’m not very good at evaluating PR — I don’t know the exact direction in this matter. For myself, I’ve decided not to use $_REQUEST and getVar(). Do you know any specific cases where this is 100 % necessary? In most cases, it’s better to use an explicit data source (GET, POST, COOKIE).

Earlier, it was said that this is a system class (for tests), and for the user, you need to use Request.

Wait for the member answers.

[Copilot]

Copilot review overview

🟡 Changes recommended

Critical and moderate findings remain around request overrides and synchronizing $_REQUEST.

Review effort: Lite
Findings: 2

Open (2)

• Preserve explicit request overrides when merging superglobals · New
• Add cookie and precedence coverage for merged request variables · New

What changed in this PR

Updates request-variable retrieval so getVar() reflects current GET/POST/COOKIE data after URI parsing.

Changes:

• Adds merged request-data retrieval.
• Refactors reusable array-fetching logic.
• Updates request and validation tests.

File	Summary
tests/​system/​Validation/​ValidationTest.php	Adjusts validation fixtures to use POST data.
tests/​system/​SuperglobalsTest.php	Tests merged request-data behavior.
tests/​system/​HTTP/​IncomingRequestTest.php	Updates request-variable fixtures.
system/​Superglobals.php	Builds merged request data.
system/​HTTP/​RequestTrait.php	Extracts reusable array-fetching logic.
system/​HTTP/​IncomingRequest.php	Uses merged data in getVar().

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rahul05ranjan]

@neznaika0 thanks for the review — and your instinct here is exactly right.

To be clear, this PR doesn't add any new reliance on $_REQUEST or getVar(). It fixes a real bug in existing code: Validation::withRequest() still calls $request->getVar(), and getVar() was reading the stale $_REQUEST. So the choice isn't "should we use getVar()" — it's "getVar() is still here and still used by withRequest(), so it must return correct data."

The direction matches what @paulbalandan suggested in #9872: instead of mutating $_REQUEST (which #10205 did and was rightly rejected), getVar() now returns a merged view of $_GET/$_POST/$_COOKIE respecting request_order. $_REQUEST is never touched. This also moves us toward eventually deprecating getVar(), since it no longer depends on $_REQUEST at all.

On "100% necessary" cases: I agree — there's no case where getVar() is strictly necessary, and explicit getGet()/getPost()/getCookie() are always clearer. But withRequest() is public API that currently routes through getVar(), and until that's changed it needs to work. Happy to open a follow-up switching withRequest() to explicit sources if maintainers prefer that direction.