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

Parameter sorting and encoding #14

Closed
novemberborn opened this issue Dec 2, 2010 · 4 comments
Closed

Parameter sorting and encoding #14

novemberborn opened this issue Dec 2, 2010 · 4 comments

Comments

@novemberborn
Copy link
Contributor

Reading through http://tools.ietf.org/html/rfc5849:

3.4.1.3.2. Parameters Normalization

The parameters collected in Section 3.4.1.3 are normalized into a
single string as follows:

  1. First, the name and value of each parameter are encoded
    (Section 3.6).
  2. The parameters are sorted by name, using ascending byte value
    ordering. If two or more parameters share the same name, they
    are sorted by their value.

However, in node-oauth:

exports.OAuth.prototype._normaliseRequestParams= function(arguments) {
  var argument_pairs= this._sortRequestParams( arguments );
  var args= "";
  for(var i=0;i<argument_pairs.length;i++) {
      args+= this._encodeData( argument_pairs[i][0] );
      args+= "="
      args+= this._encodeData( argument_pairs[i][1] );
      if( i < argument_pairs.length-1 ) args+= "&";
  }     
  return args;
}

I.e. the params are sorted before they are encoded.

@ciaranj
Copy link
Owner

ciaranj commented Dec 2, 2010

Good catch!

@novemberborn
Copy link
Contributor Author

Wow, Github website isn't showing the second part of your comment.

Regarding #7, it's possible this might result in different base strings and therefore a signature mismatch.

@ciaranj
Copy link
Owner

ciaranj commented Dec 2, 2010

Ah, yes thats because I edited it almost immediately when I realised that isn't the issue with #7 as that particular issue is about the parameter values not the parameter names, so the order should remain the same :)

@ciaranj
Copy link
Owner

ciaranj commented Dec 5, 2010

One day I'll get the hang of the Github-foo for auto-closing based on my git commits, for now though: caebbc2 resolves this issue :)

Thank you for raising it, I also found some other issues around using repeated parameters whilst writing tests for it :)

This issue 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

No branches or pull requests

2 participants