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

returnURL should be encoded in State #568

Closed
cawerkenthin opened this issue Oct 12, 2017 · 3 comments
Closed

returnURL should be encoded in State #568

cawerkenthin opened this issue Oct 12, 2017 · 3 comments

Comments

@cawerkenthin
Copy link
Contributor

Just about everywhere the spec refers to a URL, it is give a data type of "String (URL-encoded)". We do not say this in section 10 for returnURL, where the data type is just URL. This is causing some confusion for content tool developers. I propose that we change section 10 for clarity and consistency with other sections.

@fugu13
Copy link
Contributor

fugu13 commented Oct 12, 2017

I think it isn't listed as URL encoded because it shouldn't be -- it doesn't make sense to URL encode a JSON value that will be used directly in a redirect; there's no benefit, and the intended use of the value would then require every single tool URL-decode it first, since redirects don't take encoded URLs.

Doing a look around, for the most part it looks like URL-encoding is requested in appropriate places -- parameters that will go in URLs. The only exceptions, which I think might be mistakes, are the sessionid extension (does not go in URLs, is an opaque token, is placed in statements, so encoding it is quite useless), and the auth-token (the example isn't URL encoded, there's no reason to make it URL encoded as it never gets put anywhere other than an auth header and a JSON doc, and if it is URL encoded then using it requires decoding it first, every time). This one is quite likely a leftover from a time when the auth-token was placed in a URL.

@MrBillMcDonald
Copy link
Contributor

Per the Oct 13th meeting, clarifying language will be added to the specification emphasizing that the value of returnURL is a string that is NOT URL-encoded.

MrBillMcDonald added a commit that referenced this issue Nov 3, 2017
Added per Oct 13th Meeting.  add clarifying language to emphasize that returnURL  is NOT a URL-encoded string value:
MrBillMcDonald added a commit that referenced this issue Nov 3, 2017
@MrBillMcDonald
Copy link
Contributor

Closed

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

No branches or pull requests

3 participants