#40614 closed defect (bug) (fixed)
REST API: String argument for rest_do_request/rest_ensure_request does not work as expected.
Reported by: | TimothyBlynJacobs | Owned by: | kadamwhite |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | REST API | Keywords: | has-patch needs-unit-tests |
Focuses: | rest-api | Cc: |
Description
According to the rest_do_request()
php docs, a string
parameter type is accepted. Currently, this is forwarded to rest_ensure_request()
which only accepts a WP_REST_Request
object or an array. When a string argument is passed a request object is instantiated with a string value for the request attributes which is improper.
Either the docs should be updated, or, ideally, the rest_do_request
and rest_ensure_request
functions would be updated.
I could see those two functions either accepting a route path, like /wp/v2/posts
or a full url, www.example.org/wp-json/wp/v2
. Since those API functions are internal to the API instance, it makes most sense, in my mind at least, to accept a path style argument. Otherwise, there would be needless boiler plate: rest_do_request( rest_url( '/wp/v2/posts' ) )
.
The simplest implementation would then be to instantiate a WP_REST_Request
object with the given path as the second constructor argument if the $request
parameter is a string. However, this would make it impossible to quickly make a request with any query parameters attached.
That gives us a few options.
- Change
rest_ensure_request
to doWP_Rest_Request::from_url( rest_url( $request ) )
in case of a string argument. - Introduce
WP_Rest_Request::from_path
that accepts a path and does the properparse_url
andwp_parse_str
handling. - Only accept a full URL to
rest_do_request
.
If it might be confusing that rest_ensure_request
accepts a path argument instead of a URL, conceivably it could accept both and switch on the presence of a /
at the start of the string.
Happy to submit a patch to whichever makes most sense.
Attachments (4)
Change History (13)
This ticket was mentioned in Slack in #core-restapi by kosso. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
5 years ago
#5
@
5 years ago
- Milestone changed from Awaiting Review to 5.3
Seems like good low-hanging fruit to pull in for 5.3
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
5 years ago
#8
@
5 years ago
- Owner set to kadamwhite
- Resolution set to fixed
- Status changed from new to closed
In 46099:
Hmm, I wonder how this happened, maybe we changed
rest_ensure_request()
at some point, because the idea of that function was to do just this. In any case, I think it's too late to change it.I'd prefer to do the simple thing here and treat a string parameter as a route with no further parsing (that is, directly instantiate
WP_REST_Request
). More advanced usage can continue to create aWP_REST_Request
manually; I'd rather not haverest_do_request()
turn into an everything-and-the-kitchen-sink function. Alternatively, we could remove the string capability altogether and treat this as a docs bug.