-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Robust query string encoding #129
base: master
Are you sure you want to change the base?
Conversation
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.
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. |
200d074
to
a28d265
Compare
This previously relied on a hand-rolled function that tried to do its own urlencoding. This commit moves query string encoding to the standard function http_build_query.
a28d265
to
4a9b6dd
Compare
Updated |
👍 |
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. ) |
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 working: existing create_signature_string() method:
not working: proposed create_signature_string() method (notice double encoding of percent symbol from %20 spaces)
So, I found that adding:
..seems to fix it for all my requests. giving:
|
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 🙂 |
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 $param_key = $key . '[' . $param_key . ']'; // Handle multi-dimensional array did the trick :) |
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 likefilter[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 🙏.