Skip to content

[Website] Preserve plugin proxy default response headers#4157

Merged
brandonpayton merged 4 commits into
emdash/plugin-proxy-redirect-hardening-prp48from
agent/plugin-proxy-default-response-headers
Jul 24, 2026
Merged

[Website] Preserve plugin proxy default response headers#4157
brandonpayton merged 4 commits into
emdash/plugin-proxy-redirect-hardening-prp48from
agent/plugin-proxy-default-response-headers

Conversation

@ashfame

@ashfame ashfame commented Jul 23, 2026

Copy link
Copy Markdown
Member

What changed

This stacked PR adds regression coverage for filtered upstream response headers and preserves configured default response headers when a same-named upstream header is filtered out.

Why

PR #4137 buffers response headers while following redirects. The new helper marked every received header as seen before applying the response-header allowlist. A filtered upstream Content-Type therefore suppressed the configured Content-Type: application/zip fallback even though neither header reached the client.

The fix records a response header as seen only after it passes the allowlist and is forwarded.

Impact

Plugin, theme, release, and artifact downloads retain their intended ZIP content type and download metadata when upstream headers are filtered.

Commit and CI sequence

  1. Commit 7161ff666 added the regression test only. Chromium shard 2 failed with Expected: "application/zip" and Received: undefined.
  2. Commit 75c40e2b5 applied the one-line production fix. The same Chromium shard passed, including keeps a default header when the matching upstream header is filtered.

This PR is stacked on #4137 and targets its head branch.

Validation

  • Focused Playwright suite after the fix: 7 passed
  • Website lint: passed
  • PHP syntax checks for plugin-proxy.php and the test router: passed
  • GitHub Actions regression test: failed before the fix and passed after the fix

@ashfame
ashfame marked this pull request as ready for review July 23, 2026 15:37
## What changed

Remove the standalone `$pluginResponse;` expression from
`plugin-proxy.php`.

## Why

The variable is never assigned or read, and it has no declaration or
control-flow effect in PHP. The statement was introduced in 2023 with
the Gutenberg pull-request preview flow and has remained unused.
Evaluating the undefined variable can emit a warning even though
`display_errors` hides it from the HTTP response.

## Impact

There is no intended behavior change. The cleanup removes a no-op
expression and avoids a possible undefined-variable warning in server
logs.

## Stack

This PR is stacked on #4157 and targets its head branch. It should be
merged after #4157, which is itself stacked on #4137.

## Validation

- Focused plugin-proxy Playwright suite: 7 passed
- Website lint: passed
- PHP syntax check: passed

@brandonpayton brandonpayton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ashfame thank you for catching this. I left some comments and will have Codex work on them.

Comment thread packages/playground/website/playwright/e2e/plugin-proxy.spec.ts Outdated
Comment thread packages/playground/website/playwright/e2e/plugin-proxy-test-router.php Outdated
Cover both filtered and forwarded Content-Type behavior and document why the PHP and TypeScript test routers are needed.
@brandonpayton
brandonpayton merged commit 6c63ae8 into emdash/plugin-proxy-redirect-hardening-prp48 Jul 24, 2026
101 of 103 checks passed
@brandonpayton
brandonpayton deleted the agent/plugin-proxy-default-response-headers branch July 24, 2026 04:03
brandonpayton added a commit that referenced this pull request Jul 24, 2026
## What changed

This stacked PR adds regression coverage for filtered upstream response
headers and preserves configured default response headers when a
same-named upstream header is filtered out.

## Why

PR #4137 buffers response headers while following redirects. The new
helper marked every received header as seen before applying the
response-header allowlist. A filtered upstream `Content-Type` therefore
suppressed the configured `Content-Type: application/zip` fallback even
though neither header reached the client.

The fix records a response header as seen only after it passes the
allowlist and is forwarded.

## Impact

Plugin, theme, release, and artifact downloads retain their intended ZIP
content type and download metadata when upstream headers are filtered.

## Commit and CI sequence

1. Commit `7161ff666` added the regression test only. [Chromium shard 2
failed](https://github.com/WordPress/wordpress-playground/actions/runs/30019151059/job/89246998902)
with `Expected: "application/zip"` and `Received: undefined`.
2. Commit `75c40e2b5` applied the one-line production fix. [The same
Chromium shard
passed](https://github.com/WordPress/wordpress-playground/actions/runs/30020038612/job/89250060487),
including `keeps a default header when the matching upstream header is
filtered`.

This PR is stacked on #4137 and targets its head branch.

## Validation

- Focused Playwright suite after the fix: 7 passed
- Website lint: passed
- PHP syntax checks for `plugin-proxy.php` and the test router: passed
- GitHub Actions regression test: failed before the fix and passed after
the fix

---------

Co-authored-by: Brandon Payton <brandon@happycode.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants