Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CORS Proxy] Support chunked encoding when running in Apache/Nginx/etc #2114

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jan 2, 2025

Adds support for transfer-encoding: chunked on Apache, Nginx, and other web servers.

#2077 added support for transfer-encoding: chunked in a way that works on a local PHP dev server. The proxy manually chunks the output bytes and outputs each chunk's header, separator, body, and trailer.

However, it doesn't work on playground.wordpress.net because the web server there handles the chunked encoding on its own. The bytes echoed by cors-proxy.php are treated as body bytes, which messes up the response body.

This PR restricts the manual chunking to a local CLI dev server.

Testing instructions

Run cors-proxy.php in Apache or so and confirm that requesting resources served with chunked encoding works with this patch but not without it. One such URL is https://adamadam.blog/feed/.

Adds support for `transfer-encoding: chunked` on Apache, Nginx, and other web servers.

#2077 added support for `transfer-encoding: chunked` in a way that works on a local PHP dev server. The proxy manually chunks the output bytes and outputs each chunk's header, separator, body, and trailer.

However, it doesn't work on playground.wordpress.net because the web server there handles the chunked encoding on its own. The bytes echoed by cors-proxy.php are treated as body bytes, which messes up the response body.

This PR restricts the manual chunking to a local CLI dev server.

## Testing instructions

Run cors-proxy.php in Apache or so and confirm that requesting resources served with chunked encoding works with this patch but not without it. One such URL is https://adamadam.blog/feed/.
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Package][@wp-playground] CORS Proxy labels Jan 2, 2025
@adamziel adamziel changed the title [CORS Proxy] Fix transfer-encoding: chunked [CORS Proxy] Support chunked encoding when running behind Apache Jan 2, 2025
@adamziel adamziel changed the title [CORS Proxy] Support chunked encoding when running behind Apache [CORS Proxy] Support chunked encoding in Apache Jan 2, 2025
@adamziel adamziel changed the title [CORS Proxy] Support chunked encoding in Apache [CORS Proxy] Support chunked encoding when running in Apache/Nginx/etc Jan 2, 2025
Copy link
Member

@brandonpayton brandonpayton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool :) This looks reasonable, and it's interesting to learn of the need of this. I guess it's not surprising that web servers would manage their own response encoding.

// Only send chunked transfer encoding footer if we're using chunked encoding.
// We need to manually send the footer when running in the PHP built-in server
// because, unlike apache or nginx, it won't handle that for us.
if ($is_chunked_response && php_sapi_name() === 'cli-server') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to add this under the code review response...

nit: The terms of this condition are used in multiple places. Maybe it would be good to capture it in a function like should_send_as_chunked_response().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package][@wp-playground] CORS Proxy [Type] Bug An existing feature does not function as intended
Projects
Status: Inbox
Development

Successfully merging this pull request may close these issues.

2 participants