Allow HTTP PUT requests sent from Jetpack to WPCOM
#6853
Conversation
Previously, `Jetpack_Client::wpcom_json_api_request_as_blog()` only supported `GET` and `POST` verbs, but this allows supporting any method such as `PUT` or `DELETE`.
HTTP requests made by functions like `Jetpack_Client::remote_request()` generate signed body hashes for the request content. If the data is sent via the `PUT` method, however, it was ignored, causing a hash comparison failure on the server. This change allows `PUT` data to be validated as well. Just like the `POST` data, the `PUT` data is taken from the raw url-encoded input (I don't understand why), but if IS_WPCOM, we assume the data is JSON-encoded and use the decoded version.
jeherve
added
[Feature] WPCOM API
[Pri] Normal
[Status] Needs Review
| @@ -71,7 +71,17 @@ function sign_current_request( $override = array() ) { | |||
| $body = $_POST; | |||
| } | |||
| } | |||
| } else if ( 'PUT' == strtoupper( $_SERVER['REQUEST_METHOD'] ) ) { | |||
| // This is a little strange-looking, but there doesn't seem to be another way to get the PUT body | |||
| $raw_put_data = file_get_contents( 'php://input' ); | |||
There was a problem hiding this comment.
This line gave me pause, because of an earlier conversation here:
#5418 (comment)
But I do not think earlier concerns apply as this is for outgoing requests.
There was a problem hiding this comment.
Good find. While this function would also be run on incoming PUT requests, I don't think it would cause any issues since the execution would only run during the (probably very short-lived) REST API request handler.
|
I have tested a bunch for my use-case (Jetpack to WPCOM), but I'm honestly not sure how to test for other use-cases. The reason is that there are no existing That said, I still think it is correct, with one potential issue: if for some reason a At the moment I feel that that these risks are pretty small and would likely be caught and adjusted when such functionality was developed. |
roccotripaldi
added
[Status] Ready to Merge
Agreed. I'm marking this as ready to merge. Perhaps in the future we can make our hashing algorithm work better with complex PHP arrays so that it can handle anything thrown at it. |
| @@ -299,11 +299,7 @@ static function wpcom_json_api_request_as_blog( $path, $version = self::WPCOM_JS | |||
| $_path = preg_replace( '/^\//', '', $path ); | |||
|
|
|||
| // Use GET by default whereas `remote_request` uses POST | |||
| if ( isset( $filtered_args['method'] ) && strtoupper( $filtered_args['method'] === 'POST' ) ) { | |||
There was a problem hiding this comment.
Oh! Looks like I'm also fixing this little bug: strtoupper( $filtered_args['method'] === 'POST' ). That's probably supposed to read strtoupper( $filtered_args['method'] ) === 'POST' ).
There was a problem hiding this comment.
Interestingly, the only reason has worked in the past is because strtoupper(false) evals to "", which is falsy in PHP, and strtoupper(true) evals to "1", which is truthy. It would never have worked with "post" though.
matticbot
removed
the
[Status] Ready to Merge
* Changelog: initial commit for 4.9 release. * Changelog: add #6929 * Changelog: move old changelogs to changelog.txt * Readme: restore deleted release post link. The post is now live. * Changelog: add #6853 * Changelog: add #6856 * Changelog: add #6857 * Changelog: add #6884 * Changelog: add #6885 * Changelog: add #6892 * Changelog: add #6894 * Changelog: add #6898 * Changelog: add #6899 * Changelog: add #6900 * Changelog: add #6909 * Changelog: add #6927 * Changelog: add #6947 * Chagelog: add #6958 * Changelog: add #6961 * Changelog: add #6963 * Changelog: add #6965 * Changelog: add #6986 * Changelog: add #7000 * Changelog: add #7013 * Changelog: add #7015 * Changelog: add #7019 * Changelog: add #7028 * Changelog: add #6998 * Changelog: add #6999 * Changelog: add #7044 * Changelog: add #6881 * Changelog: add #6922 * Changelog: add #6940 * Changelog: add #6962 * Changelog: add #6942 * Changelog: add #6959 * Changelog: add #7018 * Changelog: add #6948 * Changelog: add #6657 * Changelog: add #7030 * Changelog: add #7048 * Changelog: add #7031 * Changelog: add #6990 * Changelog: add #6957 * Changelog: add #7027
This has two parts:
Allow any HTTP method in wpcom_json_api_request_as_blog
Previously,
Jetpack_Client::wpcom_json_api_request_as_blog()onlysupported
GETandPOSTverbs, but this allows supporting any methodsuch as
PUTorDELETE.Allow HTTP PUT data to be validated by Jetpack_Signature
HTTP requests made by functions like
Jetpack_Client::remote_request()generate signed body hashes for the request content. If the data is sent
via the
PUTmethod, however, it was ignored, causing a hash comparisonfailure on the server. This change allows
PUTdata to be validated aswell.
Just like the
POSTdata, thePUTdata is taken from the rawurl-encoded input (I don't understand why), but if IS_WPCOM, we assume
the data is JSON-encoded and use the decoded version.