Repository navigation
Harden PHP CORS proxy header filtering - #4506
Draft
brandonpayton wants to merge 2 commits into
Draft
brandonpayton wants to merge 2 commits into
brandonpayton wants to merge 2 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for the change, related issues
The PHP CORS proxy currently forwards client credentials and upstream policy headers that belong to the proxy's own connection or origin. It also emits upstream fields before seeing the complete header block, allowing a later
Connectionheader to nominate a field that has already been queued for the client.Strengthen this boundary while preserving streamed bodies, uploads, range requests, HEAD, redirect rewriting, Authorization opt-in, rate limiting, and the existing declared-size limits.
Implementation details
Connectionnomination. Buffer only upstream headers until the block is complete; continue streaming response bodies.X-Forwarded-ForandX-IP-Proxy-Typeinputs from outgoing requests without changing rate-limit identity handling.X-Spacefast-*/X-Stattic-*headers while preserving existing server/CDN control-header filtering. Protect proxy and deployment-owned headers, supplyVary: OriginandCross-Origin-Resource-Policy: cross-origin, and retain the existing origin rules for CORS and the legitimate proxy marker.Content-Lengthwhenever upstream transfer encoding is present and preserve unchanged compressed representation bytes and metadata.Compatibility changes: The newly blocked headers are intentionally no longer forwarded. An upstream
Connectionnomination of a non-identityContent-Encodingnow returns a clean 502 rather than relaying compressed bytes without the metadata needed to decode them.Testing Instructions (or ideally a Blueprint)
Run the default PHPUnit and PHP built-in-server end-to-end suites:
To also exercise Apache/FastCGI on macOS:
See the proxy README for Apache module and PHP CGI overrides. The Apache fixture enables Authorization passthrough, trusts lengths from the proxy script, flushes FastCGI output, and supplies lengths for chunked uploads; these are test fixture settings, not deployment changes.
Validated locally with PHP 8.5.8 and Apache 2.4.67: 264 unit tests (365 assertions) and 527 end-to-end assertions passed. Coverage includes mixed casing, repeated and reordered
Connectionnominations, attempts to forge proxy-owned headers, delayed streaming, fixed-length and chunked gzip bodies, conflicting transfer framing, interim responses, ranges, HEAD, and raw/chunked/wrapped/reconstructed uploads.git diff --checkalso passed.Remaining limits: Size checks retain their existing declared-length semantics rather than counting unknown-length streams. Live CDN/nginx deployment behavior has not been verified.