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

Remove x-request-issuer header on cross-origin requests #1010

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak commented Feb 7, 2024

What is this PR doing?

Restricts the x-request-issuer header added in 9f134e0 added to local requests.

We can't just add this header to all external
requests because of CORS. The browser will refuse to process
cross-origin requests with custom headers unless the server
explicitly allows them in Access-Control-Allow-Headers.

Testing Instructions

@bgrgicak bgrgicak self-assigned this Feb 7, 2024
@bgrgicak bgrgicak requested review from dd32 and adamziel February 7, 2024 09:28
@bgrgicak bgrgicak closed this Feb 7, 2024
@bgrgicak bgrgicak force-pushed the remove/x-request-issuer-header branch from 6553d5d to a38b64a Compare February 7, 2024 09:29
@bgrgicak bgrgicak reopened this Feb 7, 2024
@adamziel
Copy link
Collaborator

adamziel commented Feb 7, 2024

This breaks the site health part of wp-admin. In there, Request1 to PHP triggers wp_safe_http_get and issues Request2 to itself. However, Request2 can't acquire an exclusive lock required to serve it because that lock is already held by Request1. We need an E2E test to make something in CI flash red.

A neat solution would be to only include that header when the request domain is the same as the current document.location.host.

@adamziel adamziel changed the title Remove x-request-issuer header Only add the x-request-issuer header on self requests Feb 7, 2024
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Aspect] Browser [Aspect] Networking labels Feb 7, 2024
@adamziel adamziel merged commit 990d025 into trunk Feb 7, 2024
4 checks passed
@adamziel adamziel deleted the remove/x-request-issuer-header branch February 7, 2024 15:23
@adamziel adamziel changed the title Only add the x-request-issuer header on self requests Remove x-request-issuer header on cross-origin requests Feb 7, 2024
@adamziel adamziel mentioned this pull request Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Aspect] Browser [Aspect] Networking [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants