Skip to content

Allow for CORS requests to api.wordpress.org to pass - #1009

Closed
dd32 wants to merge 4 commits into
trunkfrom
fix/networking-api-cors
Closed

Allow for CORS requests to api.wordpress.org to pass#1009
dd32 wants to merge 4 commits into
trunkfrom
fix/networking-api-cors

Conversation

@dd32

@dd32 dd32 commented Feb 7, 2024

Copy link
Copy Markdown
Member

What is this PR doing?

Fixes CORS requests to WordPress.org, even with custom headers.

What problem is it solving?

See #1003

This is a workaround until https://make.wordpress.org/systems/2024/02/07/cors-requests-for-api-wordpress-org/ is resolved.

How is the problem addressed?

Altering the URI that fetches to api.wordpress.org make, to replace requests to /path/ to /path/index.php to allow for OPTIONS requests to reach PHP.

Testing Instructions

Untested. Make sure upon booting no networking errors are encountered.

@bgrgicak
bgrgicak self-requested a review February 7, 2024 09:00

@bgrgicak bgrgicak left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dd32 this PR doesn't work for me, I still get CORS errors when I open the plugins list.

Screenshot 2024-02-07 at 10 11 43

@dd32

dd32 commented Feb 7, 2024

Copy link
Copy Markdown
Member Author

@bgrgicak Thanks! I wasn't actually able to properly test this.

I guess this goes back to #933 then - That custom header shouldn't be included with all outgoing HTTP requests.

@bgrgicak

bgrgicak commented Feb 7, 2024

Copy link
Copy Markdown
Collaborator

Looks like this custom header is causing the issue.

If I remove it on trunk, requests work again.

@bgrgicak

bgrgicak commented Feb 7, 2024

Copy link
Copy Markdown
Collaborator

@dd32 this PR should fix the issue #1010

@adamziel

adamziel commented Feb 7, 2024

Copy link
Copy Markdown
Collaborator

Thank you so much @dd32! I somehow assumed the CORS setup on w.org must have changed when it was a Playground PR all along. 🙌 I'll go ahead and merge #1010 to remove the custom header, but this PR still seems to makes sense regardless or #1010. Would you recommend merging it anyway?

@adamziel adamziel mentioned this pull request Feb 8, 2024
@dd32

dd32 commented Feb 21, 2024

Copy link
Copy Markdown
Member Author

Would you recommend merging it anyway?

Kind of; but at the same time, if things are working right now... might be better to just leave it as-is.

@dd32 dd32 closed this Feb 21, 2024
@adamziel adamziel mentioned this pull request Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants