Skip to content

Add OAUTH_SIGNATURE EncodeSet #148

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

Closed
wants to merge 3 commits into from
Closed

Add OAUTH_SIGNATURE EncodeSet #148

wants to merge 3 commits into from

Conversation

alicemaz
Copy link

This character set is defined in rfc5849
https://tools.ietf.org/html/rfc5849#section-3.6
and is used to encode the signature base string and
and the "Authorization" header field

relevant bit of the oauth 1.0 rfc is:

  • (ALPHA, DIGIT, "-", ".", "_", "~") MUST NOT be encoded.
  • All other characters MUST be encoded.

tested with all ascii values 32-126 in both the signature and the header

Review on Reviewable

This character set is defined in rfc5849
https://tools.ietf.org/html/rfc5849#section-3.6
and is used to encode the signature base string and
and the "Authorization" header field
@untitaker
Copy link
Contributor

Since this part of the OAuth 1.0 RFC is just refering to unreserved in the URI RFC, I think a name should be chosen that isn't specific to OAuth.

I'm currently a bit confused about the naming scheme that is used for encoding sets, but it seems UNRESERVED_ENCODING_SET should be fine (I think?)

@alicemaz
Copy link
Author

updated. agree it is a better name. I'm not sure what the comment should be changed to, if anything, to fit with the UNRESERVED name while keeping the "is used for" phrasing.

incidentally I noticed HTTP_VALUE_ENCODE_SET is not listed in the docs (https://servo.github.io/rust-url/url/percent_encoding/index.html), though it does exist in the copy of the file in the repo (https://github.com/servo/doc.servo.org/blob/gh-pages/url/percent_encoding/index.html#L89), but I don't know whether this is in error or where it would make sense to report this.

@untitaker
Copy link
Contributor

Not sure either. ISTM that unreserved is often used in contexts where a proper URL parsing library (like this one) would do a better job at interpreting the URL correctly.

  • The only usecase I really have is to normalize URLs using decode(url, UNRESERVED) (as specified in the RFC), but in Rust, rust-url already takes care of that.
  • The other thing I've just learned about is that it's used in OAuth 1.0

So perhaps:

This encode set is used for URI normalization. It can also be used to implement other RFCs which rely on the definition of unreserved.

@untitaker
Copy link
Contributor

where a proper URL parsing library (like this one) would do a better job at interpreting the URL correctly.

Ironically rust-url doesn't do percent-encoding normalization, so I've filed #149

@alicemaz
Copy link
Author

thank you, updated.

@SimonSapin
Copy link
Member

Rather than add this (and later another set for another use case, and another…) I’d like to do something like #151 so you could define this in your own crate. What do you think?

@untitaker
Copy link
Contributor

@SimonSapin This particular set might actually be needed depending on your response to #149

@SimonSapin
Copy link
Member

@untitaker I don’t understand how that particular set is relevant to #149, but that should probably be discussed there since this PR is originally about supporting OAuth.

@alicemaz
Copy link
Author

alicemaz commented Dec 6, 2015

@SimonSapin if I understand correctly, this would allow me to call the define_encode_set! macro once to create the ruleset in my program, then pass the result to the encoder as needed? that would fix my use case. specifically, all I need is to be able to do this exact encoding to arbitrary strings so when I hash them after it comes out right.

I don't know enough on the topic to have an opinion on whether this particular set ought to be in the lib proper for normalization in-browser and leave it to your judgement.

@SimonSapin
Copy link
Member

if I understand correctly, this would allow me to call the define_encode_set! macro once to create the ruleset in my program, then pass the result to the encoder as needed?

Yes, exactly.

whether this particular set ought to be in the lib

My opinion of the scope of rust-url is "things defined in https://url.spec.whatwg.org/"

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #176) made this pull request unmergeable. Please resolve the merge conflicts.

@SimonSapin
Copy link
Member

1.0.0 will be published tomorrow. It has a mechanism for defining your own encode set:
https://servo.github.io/rust-url/url/macro.define_encode_set!.html

@SimonSapin SimonSapin closed this Apr 20, 2016
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.

4 participants