Skip to content
This repository has been archived by the owner on Apr 21, 2020. It is now read-only.

Fix Signature Encoding Scheme for Parameters #6

Merged
merged 1 commit into from
Jun 23, 2015
Merged

Fix Signature Encoding Scheme for Parameters #6

merged 1 commit into from
Jun 23, 2015

Conversation

jekhokie
Copy link
Contributor

Since the OAuth Signature base is calculated and follows the RFC
requirement to encode parameters prior to doing anything else (sorting,
etc), it is necessary to pass a decoded version of all parameters to the
function, and then encode once. Removed extraneous flip-flopping of
encodings, which cause an issue if parameters passed have special
characters (i.e. spaces, percent symbols, etc).

Since the OAuth Signature base is calculated and follows the RFC
requirement to encode parameters prior to doing anything else (sorting,
etc), it is necessary to pass a decoded version of all parameters to the
function, and then encode once. Removed extraneous flip-flopping of
encodings, which cause an issue if parameters passed have special
characters (i.e. spaces, percent symbols, etc).
@ryanbreen
Copy link
Contributor

we need some test cases for these. I'm happy to write them if you don't have time.

@jekhokie
Copy link
Contributor Author

Hi Ryan, would you mind writing the test cases? I haven't had the time to do so yet.

@ryanbreen
Copy link
Contributor

Will do.

ryanbreen added a commit that referenced this pull request Jun 23, 2015
Fix Signature Encoding Scheme for Parameters
@ryanbreen ryanbreen merged commit 1ec0d4e into Cimpress-MCP:master Jun 23, 2015
@@ -40,9 +40,9 @@ function normaliseRequestParams(argument_pairs) {
continue;
}
**/
args += sprintf("%s=%s", argument_pairs[i][0], argument_pairs[i][1]);
args += sprintf("%s%%3D%s", argument_pairs[i][0], argument_pairs[i][1]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I look at it, I think this line is wrong (both before and after edit). A query param with no value and no equals is common and (I think) legal, but this code assumes there's always an equals. This needs more testing and potentially a fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, RFC is pretty clear on this point: http://oauth.net/core/1.0a/#anchor13

theopak added a commit that referenced this pull request Oct 7, 2015
Make test less vulnerable to differences in output between versions.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants