Opened 6 years ago
Closed 6 years ago
#45753 closed defect (bug) (fixed)
REST API: Allow header not set when OPTIONS request is made to a route that contains URL params
Reported by: | noisysocks | Owned by: | kadamwhite |
---|---|---|---|
Milestone: | 5.2 | Priority: | normal |
Severity: | normal | Version: | 5.0.2 |
Component: | REST API | Keywords: | has-unit-tests has-patch needs-testing |
Focuses: | Cc: |
Description
The REST API does not correctly set an Allow
header when an OPTIONS
request is made to a route that contains a URL param, e.g. /wp/v2/posts/(?P<id>[\d]+)
. This affects the posts
and blocks
routes.
Steps to reproduce:
- Create and publish a post. Note the ID of the post.
- As an admin, make an
OPTIONS http://local.wordpress.test/wp-json/wp/v2/posts/123
request, replacinglocal.wordpress.test
with the URL of your testing environment, and123
with the ID you noted in (1).
Expected result:
The response headers should contain an Allow
header:
Allow: GET, POST, PUT, PATCH, DELETE
Actual result:
The response is missing an Allow
header.
Possible cause:
This is possibly to do with rest_handle_options_request
not calling $request->set_url_params()
unlike dispatch
which correctly parses and sets URL parameters.
Attachments (3)
Change History (11)
This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.
6 years ago
This ticket was mentioned in Slack in #core by killua99. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.
6 years ago
@
6 years ago
WIP: Currently it does what the ticket said to do. On my TODO list are, the Unit Test and perhaps some minor tweaks. I would like to get some help with the authentication level on my local test I'm not able to do a Basic authentication.
This ticket was mentioned in Slack in #core-restapi by killua99. View the logs.
6 years ago
@
6 years ago
WIP: Patch with simple Unit Test. It doesn't test against a login user with capabilities edit_posts.
@
6 years ago
Add test assertion for logged-in user, mirror test to attachments controller, and run filters in tests instead of causing side-effects within rest_handle_options_request.
#7
@
6 years ago
- Keywords has-unit-tests has-patch needs-testing added; needs-patch removed
- Owner changed from killua99 to kadamwhite
- Status changed from assigned to accepted
Thanks for the patch, @killua99 ! I've updated it with a test for a logged-in user, and duplicated it to another controller as a spot-check that the fix is applied at the base API level and not specifically to posts alone.
I noticed that our other extant tests for Allow headers (see test_allow_header_sent
) were manually triggering the rest_post_dispatch
filter in order to apply the desired Allow
header. By adding a call to that filter to the unit tests — mirroring the behavior within the controller when an actual request is being dispatched — I was able to remove the call to rest_send_allow_header
within rest_handle_options_request
, eliminating a benign but unnecessary side-effect. The rest of the patch looks great as-is.
This could use one other pair of eyes and a manual test, then I think we can land it!
@killua99 has agreed to take the first pass at a patch for this.