WP_REST_Request::get_param() doesn't work for getting body params on DELETE requests

[mnelson4]

You cannot pass a request body on DELETE requests, although it seems to be admissible (see ​http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request), and when deleting posts it should be possible (eg you can provide a "force" argument, but it gets ignored unless it's included in the querystring.)

The problem seems to be that WP_REST_Request::get_parameter_order() doesn't list DELETE as one of the methods that can pass a body in its variable $accepts_body_data.

Is the idea to just always provide the DELETE's parameters in the querystring? (eg that's how I saw it done here: ​https://github.com/WP-API/WP-API/issues/789#issuecomment-73986884). If so I guess that's ok although I couldn't find it documented. ...But also, just adding DELETE to the $accepts_body_data array is an easy change too, right? From my own testing that change got it to work how I had hoped.

[jnylen0]

Nice find, @mnelson4 - I think it's just an oversight, and I'd like to get it fixed. Can you put up a patch and unit tests?

[swissspidy]

According to the ​documentation, "Body parameters can and should be used for POST, PUT, and DELETE requests."

However, I recall some discussion where it was said it's not supported because many servers don't support it, or something like that. The only thing I could find right now was ​https://wordpress.slack.com/archives/core-restapi/p1455121876002422 though.

[mnelson4]

Can you put up a patch and unit tests?

sure @jnylen0 

I recall some discussion where it was said it's not supported because many servers don't support it, or something like that

@swissspidy I dunno. Do you think I should hold off making a patch until there is some more discussion?

[jnylen0]

Replying to swissspidy:

it's not supported because many servers don't support it, or something like that. The only thing I could find right now was ​https://wordpress.slack.com/archives/core-restapi/p1455121876002422 though.

This isn't a good argument not to support something, IMO. We support plenty of functionality that some servers may not allow, usually with appropriate fallbacks and documentation. One example is the _method parameter documented ​here.

Replying to mnelson4:

Do you think I should hold off making a patch until there is some more discussion?

I would go ahead and submit the patch, that will help us get it fixed more quickly.

[swissspidy]

@jnylen0 My comment wasn't an argument against this. I wanted to add some context/background so people know why something is that way.

[mnelson4]

allows you to pass request body on DELETE requests over the REST API

[jnylen0]

Whitespace in the tests looks off, and we can use a ​PHPUnit data provider to make the tests cleaner.

Neither of those are difficult to fix. I'm not sure yet whether we can do it, but I'm going to look at getting this into 4.7.3.

[mnelson4]

Whitespace in the tests looks off,

Oh right, I think I used spaces instead of tabs. I can fix

we can use a PHPUnit data provider to make the tests cleaner.

Oh I actually wasn't even aware of data providers. Makes a lot of sense. This is one reason I'm glad to participate in core!

[mnelson4]

fixes spacing issue from previous commit

[mnelson4]

Use a data provider for rest request unit test

[mnelson4]

adds a PHPdoc to the data provider method which I forgot in the previous commit

[jnylen0]

Unify previous patch files; further tests cleanup

[jnylen0]

39933.5.diff​ looks good and tests out well for me. I merged the previous patch files into one (easier to deal with this way), did a bit more cleanup, and skipped the PHPDoc on the data provider method as I don't think it's necessary.

[jnylen0]

A little more test cleanup

[jnylen0]

39933.6.diff​ is a slightly cleaner patch which avoids modifying the assert statement to include a message. This isn't really necessary because PHPUnit will still provide the failing request method for us:

1) Tests_REST_Request::test_non_post_body_parameters with data set #2 ('DELETE')
Failed asserting that null matches expected 'bar'.

[jnylen0]

In 40105:

REST API: Correctly parse body parameters for DELETE requests.

DELETE was inadvertently omitted from the list of non-POST HTTP methods that should be able to accept body parameters. Parameters passed to DELETE requests as JSON are already parsed correctly; this commit fixes application/x-www-form-urlencoded parameters as well.

Props mnelson4.
Fixes #39933.

[jnylen0]

Reopening for backport to 4.7 branch.

[mnelson4]

39933.6.diff​ is a slightly cleaner patch which avoids modifying the assert statement to include a message. This isn't really necessary because PHPUnit will still provide the failing request method for us:

ah, cool. Also wasn't aware data sets provided that. Great.

[SergeyBiryukov]

In 40113:

REST API: Correctly parse body parameters for DELETE requests.

DELETE was inadvertently omitted from the list of non-POST HTTP methods that should be able to accept body parameters. Parameters passed to DELETE requests as JSON are already parsed correctly; this commit fixes application/x-www-form-urlencoded parameters as well.

Props mnelson4.
Merges [40105] to the 4.7 branch.
Fixes #39933.