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

Send WordPress API requests directly. #933

Merged
merged 5 commits into from
Jan 15, 2024
Merged

Send WordPress API requests directly. #933

merged 5 commits into from
Jan 15, 2024

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Jan 11, 2024

What is this PR doing?

Sends HTTP requests directly to api.wordpress.org instead of proxying them.

For #916 while looking at the plugin/theme info, I didn't see any reason for them to be proxied, as the endpoints being used there supported CORS requests.

While testing however, I found a number of endpoints that were in use that did not have CORS headers, and as a result, I've added them:

  • POST /plugins/update-check/1.1/
  • POST /themes/update-check/1.1/
  • POST /core/browse-happy/1.1/
  • GET /core/serve-happy/1.0/
  • POST /core/version-check/1.7/

For some reason, while /core/version-check/1.7/ has CORS headers (already) it's still performing an preflight (OPTIONS) request which fails. I'm not sure why that's happening.

I've had to adjust the handling for POST requests as well, as fetch() requires Content-Type: application/x-www-form-urlencoded to be set, which is just assumed/set by PHP.
I'm pretty sure there's a nicer way to handle the data.headers being an array, but this worked for me.

What problem is it solving?

I hope this resolves #916 by removing the need to proxy it, avoiding any rate limits

How is the problem addressed?

See above

Testing Instructions

Unsure

dd32 added 2 commits January 11, 2024 17:27
…' header to form-urlencoded, which is not usually specified in PHP.
@dd32
Copy link
Member Author

dd32 commented Jan 11, 2024

For some reason, while /core/version-check/1.7/ has CORS headers (already) it's still performing an preflight (OPTIONS) request which fails. I'm not sure why that's happening.

Ahh.. because we're sending custom headers, which is not supported by CORS without a preflight request.

https://github.com/WordPress/wordpress-develop/blob/94b70f1ae065f10937c22b2d4b180ceade1ddeee/src/wp-includes/update.php#L195-L198

Even if I allow those headers on the API endpoint, because we don't support OPTIONS requests (I believe nginx is eating the request before it gets to PHP, probably in the reverse proxying we use) the request will never pass either.

As a hacky workaround, the fetch code could just only use cors-whitelisted headers to remove the need for the preflight.. I think that would be acceptable.. Otherwise just skipping those wp_* headers would probably be worthwhile (which the next commit has done)

@adamziel
Copy link
Collaborator

Thank you @dd32! This looks solid. I took the liberty of fixing the lint errors and adjusting the code slightly to make the linter happy and address what you said about the arrays here, and I'd love to merge this as soon as the CI checks pass. Thank you!

I'm pretty sure there's a nicer way to handle the data.headers being an array, but this worked for me.

@adamziel adamziel merged commit 388c0c1 into WordPress:trunk Jan 15, 2024
5 checks passed
@dd32 dd32 deleted the try/direct-api branch January 16, 2024 02:02
brandonpayton pushed a commit that referenced this pull request Jun 17, 2024
## Motivation for the change, related issues

api.WordPress.org/core/version-check now supports preflight requests,
and so we can remove these header deletions that was to resolve CORS
issues

See #933 for where this was added.

## Testing Instructions (or ideally a Blueprint)

The following blueprint should not have a CORS fetch failure in browser
console.


https://playground.wordpress.net/?mode=seamless#{%22preferredVersions%22:{%22php%22:%228.0%22,%22wp%22:%226.5%22},%22phpExtensionBundles%22:[%22kitchen-sink%22],%22features%22:{%22networking%22:true},%22landingPage%22:%22/wp-admin/update-core.php%22,%22steps%22:[{%22step%22:%22login%22,%22username%22:%22admin%22,%22password%22:%22password%22}]}
bgrgicak pushed a commit that referenced this pull request Jun 20, 2024
## Motivation for the change, related issues

api.WordPress.org/core/version-check now supports preflight requests,
and so we can remove these header deletions that was to resolve CORS
issues

See #933 for where this was added.

## Testing Instructions (or ideally a Blueprint)

The following blueprint should not have a CORS fetch failure in browser
console.


https://playground.wordpress.net/?mode=seamless#{%22preferredVersions%22:{%22php%22:%228.0%22,%22wp%22:%226.5%22},%22phpExtensionBundles%22:[%22kitchen-sink%22],%22features%22:{%22networking%22:true},%22landingPage%22:%22/wp-admin/update-core.php%22,%22steps%22:[{%22step%22:%22login%22,%22username%22:%22admin%22,%22password%22:%22password%22}]}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxied network requests for plugin/theme api fail when using Firefox UA string
2 participants