Opened 7 years ago
Last modified 20 months ago
#43428 new enhancement
Improve CORS headers sent to REST Api requests
Reported by: | andrei.igna | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | REST API | Keywords: | |
Focuses: | rest-api | Cc: |
Description
Currently some CORS headers are not sent correctly for REST API requests. This doesn't break anything yet, but can be improved for better performance.
The problematic headers are these:
Access-Control-Allow-Headers
andAccess-Control-Allow-Methods
need to be sent just to preflight requests, meaning just on requests with OPTIONS method. Now they are sent back on any REST API request, which is unnecessaryAccess-Control-Allow-Methods
doesn't need to list the OPTIONS method. Preflight requests are made to check what's allowed, and OPTIONS requests are allowed by default. Current setup says something like 'I allow myself', as the preflight request is already a OPTIONS request
There's more detailed info here https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS, but the relevant part is this (it doesn't have an anchor for direct link):
(Note: as described below, the actual POST request does not include the Access-Control-Request-* headers; they are needed only for the OPTIONS request.)
The proposed changes are:
- Move header
Access-Control-Allow-Headers
fromserve_request
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/class-wp-rest-server.php#L239 torest_send_cors_headers
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api.php#L536 making it easier to control the CORS headers from only one place, with one hook - Remove OPTIONS keyword from
Access-Control-Allow-Methods
- Send
Access-Control-Allow-Headers
andAccess-Control-Allow-Methods
only on request with OPTIONS method (preflight), as it's the place where they are necessary - While sending the Allow headers, include
Access-Control-Max-Age: 600
for caching the preflight requests, thus improving the performance for next requests (less requests between browser & server when requesting same data). Timing can be changed - Optional - Include
Content-Disposition
inAccess-Control-Allow-Headers
, enabling direct file upload (requested in other tickets as well)
Why?
To follow recommended specs for CORS and make the REST API work as expected with web apps
All proposed changes were tested in the last few months on a fleet of ~500 websites with different web clients for managing WordPress data. Everything works fine
Change History (6)
This ticket was mentioned in Slack in #core-restapi by schlessera. View the logs.
7 years ago
#3
@
7 years ago
@andrei.igna Regarding your first point, I agree that this seems to be what the spec is stating.
However, regarding your second point, it looks like OPTIONS
is also included in the examples that the spec shows.
#4
@
7 years ago
Yes, there seems to be a small mistake on MDN docs about this. Probably will be updated shortly there too
OPTIONS
can be added in Allow response header to let the client know this method is available, but is unnecessary in Access-Control-Allow-Headers
Access-Control-Allow-Headers
is a response header found only in OPTIONS
requests, so it can't say that it allows itself after the request has been served
It always bugs me when things are not done properly and others take that as a reference..