Skip to content
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

Fix signature for params with special chars #79

Closed

Conversation

thiago-negri
Copy link
Contributor

Parameters with special characters that needs encoding should be encoded twice:

  1. Encode as they are part of a URL, so they must be encoded;
  2. Encode to join as the base string for signature.

See #34

Parameters with special characters that needs encoding should be encoded
twice:

1. Encode as they are part of a URL, so they must be encoded;
2. Encode to join as the base string for signature.

See WP-API#34
@rmccue
Copy link
Member

rmccue commented Nov 22, 2015

Hi, thanks for the PR, and apologies for the delay! Can you give an example of a URI that this fixes?

@thiago-negri
Copy link
Contributor Author

No problem. I'm trying to fetch from memory, but it is kind of hard. Sorry for not adding more information to it.

I guess that basically any URI that uses special characters that needs encoding in query strings, for example to search for the first five posts that contains "M&M" via WP-API, the URI would be something like this:

  • Base URI: https://example.com/wp-json/wp/v2/posts
  • Query:
    • filter[s] = M&M
    • posts_per_page = 5

Rendering:

https://example.com/wp-json/wp/v2/posts?filter%5Bs%D=M%26M&posts_per_page=5

Three characters need encoding in the query: [, ] and & (from M&M). So, the original query string is this:

filter%5Bs%D=M%26M&posts_per_page=5

The OAuth uses an "inner encoded base URI" to calculate the signature, so the & to separate the query names needs to be encoded too, resulting in a double-encoded query string:

filter%255Bs%25D%3DM%2526M%26posts_per_page%3D5

Without my change, it builds the original query string like this, as it does not encodes each argument:

filter[s]=M&M&posts_per_page=5

This query string is incorrect, as the filter[s] would result in being only M instead of M&M.

As I'm just recalling all this from memory, I may have missed something. Hope this helps! :)

duncanjbrown added a commit to duncanjbrown/OAuth1 that referenced this pull request Mar 4, 2016
This change was originally proposed by thiago-negri in PR WP-API#79.
@duncanjbrown
Copy link
Contributor

I need this fix for my project, so I've added it to a (non-conflicting) branch of my own. @thiago-negri, would you mind if I re-raised this PR?

@thiago-negri
Copy link
Contributor Author

@duncanjbrown Feel free to do so. :)

@thiago-negri
Copy link
Contributor Author

This PR contains conflicts and a new version without conflicts is proposed on #127.
Please, continue the discussion there.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants