OAuth1 icon indicating copy to clipboard operation
OAuth1 copied to clipboard

Robust query string encoding

Open duncanjbrown opened this issue 8 years ago • 7 comments

The plugin previously relied on a hand-rolled function that tried to do its own urlencoding.

This meant it broke on params with special chars like [] (issue #34).

A fix was previously proposed by @thiago-negri at #79. This fixed signatures for one-dimensional arrays like filter[day]=10 but did not work for 2D arrays like filter[post__not_in][]=100&filter[post__not_in][]=200.

This commit moves query string encoding to the standard php function http_build_query. It also double-encodes the generated query because it has to, to work with the rest of the plugin. I wonder if #65 may fix that?

I have some tests for this function—will attach if you give me somewhere to put them :pray:.

duncanjbrown avatar Mar 10 '16 14:03 duncanjbrown

Awesome. :)

Would you mind to change code a bit just to follow calling convention of the rest of the code?

Parameters should have a space to separate from parens.

http_build_query( $params );
preg_replace( '/%5B[0-9]+%5D/simU', '%5B%5D', $query );

Also, it would greatly help future developers if you could extend the "double-encode" comment giving a reason to why it has to do it.

thiago-negri avatar Mar 10 '16 15:03 thiago-negri

Updated

duncanjbrown avatar Mar 10 '16 15:03 duncanjbrown

👍
Thanks! This fixed my issue when using &filter[cat]=n

kosso avatar Apr 09 '16 16:04 kosso

Hmm.. however it seems to have broken media uploads. (I will double-check how my client is doing things, but it was working fine with the previous method. )

kosso avatar Apr 10 '16 12:04 kosso

update: OK. It appears my client is fine. Debugging this method compared to the existing methods shows some double-encoding of spaces in the text.

eg: Let's say I'm uploading a media file with the caption uploaded by kosso

working: existing create_signature_string() method:

caption%3Duploaded%2520by%2520kosso%26oauth_consumer_key%3D...

not working: proposed create_signature_string() method (notice double encoding of percent symbol from %20 spaces)

caption%3Duploaded%252520by%252520kosso%26oauth_consumer_key%3D...

So, I found that adding:

$replaced = preg_replace( '/%25/simU', '%', $replaced );

..seems to fix it for all my requests.

giving:

public function create_signature_string( $params ) {
    $query = http_build_query( $params );
    // http_build_query will attach numeric indices for array values, eg
    // filter[post__not_in][0]=1 instead of filter[post__not_in][]=1.
    //
    // Clients issue requests in the form filter[post__not_in][]=1 so
    // we should compare against that. This regex will strip out
    // the numeric indices.
    //
    // cf. http://php.net/manual/en/function.http-build-query.php
    // cf. http://stackoverflow.com/a/11996686/751089
    $replaced = preg_replace( '/%5B[0-9]+%5D/simU', '%5B%5D', $query );
    // http_build_query has urlencoded the parameters, but our calling function
    // expects a double-encoded return value
    // replace encoded percent symbols. 
    $replaced = preg_replace( '/%25/simU', '%', $replaced );
    return urlencode( $replaced );
}

kosso avatar Apr 10 '16 12:04 kosso

Thanks @kosso. I'm afraid I'm away this week so can't reply properly - sorry! Don't mind if you want to re-raise with commits of your own.

@rachelbaker, would you be open to a PR adding unit tests to the plugin? Then all could be robust and reliable 🙂

duncanjbrown avatar Apr 10 '16 21:04 duncanjbrown

Had a similar issue with the Wordpress Example OAuth Client which uses (an old version... of) https://github.com/thephpleague/oauth1-client/, see this issue about nested arrays.

Just wanted to say that to fix the brackets encoding problem, simply changing class-wp-rest-oauth1.php line 736 to:

$param_key = $key . '[' . $param_key . ']'; // Handle multi-dimensional array

did the trick :)

francoisjacquet avatar Jan 19 '17 21:01 francoisjacquet