Opened 5 years ago
Closed 5 years ago
#49147 closed defect (bug) (fixed)
Incorrect formatting of _links on OPTIONS request to embedded collections
Reported by: | nsundberg | Owned by: | |
---|---|---|---|
Milestone: | 5.4 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | REST API | Keywords: | good-first-bug has-patch has-unit-tests |
Focuses: | Cc: |
Description
When sending an OPTIONS
request to a non-variable route (eg. /wp/v2/posts
) with ?_embed=true
and WP_DEBUG
enabled the following warning appears (example from 5.3.2):
Warning: Invalid argument supplied for foreach() in /wp-includes/rest-api/class-wp-rest-server.php on line 569
This is caused by the following lines which sets the link as a string instead of an associative array with a href
key, like the embed_links
method expects: class-wp-rest-server.php:L1216-L1218
$data['_links'] = array(
'self' => rest_url( $route ),
);
Expected:
$data['_links'] = array(
'self' => array(
'href' => rest_url( $route ),
),
);
Was about to submit a patch but when looking into it I got a bit unsure if the _links
property even should be included in these responses. From what I can tell no other endpoints include the _links
property in the response for OPTIONS
requests, but please advise.
Attachments (7)
Change History (22)
#1
@
5 years ago
- Keywords needs-refresh good-first-bug added
- Milestone changed from Awaiting Review to 5.4
- Version changed from 4.5 to 4.4
#3
follow-up:
↓ 9
@
5 years ago
Thanks for the patch @nsundberg. There should be another layer of a wrapping array around the wrapping you added. In other words, each relation should be an array of links, where each link is an array with an href
key.
For example the following JSON is expected.
{ "_links": { "self": [ { "href": "https://" } ] } }
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
5 years ago
#6
@
5 years ago
- Keywords needs-unit-tests added
Thanks for the patch @nsundberg! Do you want to try tackling a unit test for this?
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
5 years ago
#9
in reply to:
↑ 3
@
5 years ago
Replying to TimothyBlynJacobs:
Thanks for the patch @nsundberg. There should be another layer of a wrapping array around the wrapping you added. In other words, each relation should be an array of links, where each link is an array with an
href
key.
For example the following JSON is expected.
{ "_links": { "self": [ { "href": "https://" } ] } }
Hi, I was just wondering about the array wrappers for the self
key.
I skimmed through:
https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/
and
http://stateless.co/hal_specification.html
https://tools.ietf.org/html/draft-kelly-json-hal-08
but I didn't see a similar self
array HAL examples there, just "self": { "href" :"https://..."
}
So out of curiosity, is this self
array wrapping a special WordPress implementation? Thanks.
#10
in reply to:
↑ 8
@
5 years ago
Replying to johnwatkins0:
Thanks for the unit test in 49147.3.diff.
I was just wondering about the GET
method test there:
Shouldn't the OPTIONS
method be targetted specifically here, according to the ticket's title?
ps: the @ticket 49147
annotation is missing in 49147.3.diff and array()
should be used instead of the array alias []
according to the code standard.
#11
@
5 years ago
Thanks for looking, @birgire . I've added 49147.5.diff with updates based on your feedback.
#12
@
5 years ago
@birgire We don't treat self
special, the REST API allows for any link relation to have multiple links for it so we enforce that each link relation is an array, even if it would only ever have one link.
Thanks for adding the tests @johnwatkins0! Instead of creating a new WP_REST_Server
can we use rest_get_server()
here?
Thanks for the ticket @nsundberg!
I think it'd be best to fix the link to be an associate array as you suggested. I think it'd be a good separate enhancement to add those
self
links to the OPTIONS response.