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

Signatures - Clarify usage of plus character when encoding query parameters #2641

Closed
robelcik opened this issue Sep 22, 2023 · 3 comments
Closed

Comments

@robelcik
Copy link

robelcik commented Sep 22, 2023

Hello,

In the section 2.2.8. Query Parameters we can read that:

Encode the nameString or valueString using the "percent-encode after encoding" process defined by the "application/x-www-form-urlencoded serializing" section of [HTMLURL], which results in an [ASCII] string.

As I understand the referred specification, spaces should be replaced by plus character, which is indicated by the last word below, which is "true":
(note, that it's different than encoding described in RFC 3986)

Let value be the result of running percent-encode after encoding with encoding, tuple’s value, the application/x-www-form-urlencoded percent-encode set, and true.

However, the example in 2.2.8. shows that "+" characters are replaced by "%20".

Could you please, clarify it?

Also, it would be nice, I think, to add more examples. For example "raw" GET requests containing sequences like "-", "%2D", "%48".

Also, path and query encoding could be mentioned in @target-uri and @request-target. But I'm not sure whether that is relevant.

Thank you in advance for reply.

Regards,
Robert

@jricher
Copy link
Contributor

jricher commented Sep 29, 2023

Hi Robert - The signatures spec is with the RFC editor and the text is, for all intents and purposes, final and will not be changed. To answer your specific questions though:

The canonical encoding from HTMLURL's query parameters doesn't use "+" for spaces, it uses "%20". However, it's acceptable to parse a "+" as a space. Consequently, to get to a canonical form, the signer and verifier are required to take the nameString and valueString values and first parse them into raw values then re-encode them, so that corner cases like this can be handled in a consistent manner. The example in question was specifically chosen to show this very property.

More examples are always nice, but in the end you have to balance the length of the document with the number of examples you include. One thing that I'd like to do though is put more examples on the playground website, https://httpsig.org , for people to play with directly, and we can show some of that encoding in play from a real library.

The target-uri and request-target components don't apply any additional encoding beyond what's specified, so what's in the text is what's expected. That means that a URL or query coming in with a "+" treats that as a "+" and not a space, since since that's the character that's actually passed in.

I hope this helps clarify things. I am closing the issue because the signatures specification is not in a position to take any additional changes.

@jricher jricher closed this as completed Sep 29, 2023
@robelcik
Copy link
Author

robelcik commented Oct 2, 2023

Hello!

Thank you for explanation. Now I understand your intentions, 😃

However, I'd like to nitpick a bit. I asked the initial question because I was confused. In section 2.2.8., there is a reference to "application/x-www-form-urlencoded serializing". That section says, as far as I understand, that spaceAsPlus should be true.
image

When looking at section "Percent-encoded bytes", one can see that default value of spaceAsPlus is false which, I assume, is what you refer to speaking of "canonical encoding".
image

I was thinking that someone could get confused as I was, 🤓

Regards,
Robert

@jricher
Copy link
Contributor

jricher commented Oct 2, 2023

Yes, the intent in the text is to refer to the percent encode after encoding with the space as plus set to false, its default value. I can understand the confusion here, so I'll see if we can add any clarifying text to the section during final RFC editor review, whenever that happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants