Skip to content

HttpRequest.path should preserve URL encoding #7464

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
nex3 opened this issue Dec 17, 2012 · 7 comments
Closed

HttpRequest.path should preserve URL encoding #7464

nex3 opened this issue Dec 17, 2012 · 7 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Milestone

Comments

@nex3
Copy link
Member

nex3 commented Dec 17, 2012

HttpRequest.path is un-URL-encoded. For example, a request to "/foo%2Fbar" will have a path of "/foo/bar".

@madsager
Copy link
Contributor

Nathan, could you provide a bit more context. When is it a problem for you that you get the actual string instead of the URL encoding?


Set owner to @sgjesse.

@nex3
Copy link
Member Author

nex3 commented Dec 18, 2012

This is a problem because it's impossible to distinguish between characters that mean something in the URL (e.g. "/") and characters that are just encoding data (e.g. "%2F"). A specific example is a GET request to "/package/foo%2Fbar"; this should return data about the "foo/bar" package rather than trying to look up the url "/package/foo/bar".

@sgjesse
Copy link
Contributor

sgjesse commented Dec 19, 2012

Thanks for clarifying. Of cause we need to handle the URL encoding differently. However just making HttpRequest.path the unencoded path seems to simple. How about having a canonicalization algorithm for the path.

As far as I can see we only need to treat %20 (space), %2F (/) and %3F (?) specially.

The path part of the URI is up to, and not including the first literal ? (not %3F).

The canonicalized path is the URL-decoded path except that:

  %20 (space) is decoded to +
  %2F (/) is not decoded
  everything else is decoded (including %3F (?))

Examples

  /123/456 => /123/456
  /1%32%33/%345%36 => /123/456
  /1/2/3 => /1/2/3
  /1%2F2%2F3/%345%36 => /1%2F2%2F3/456
  /1+2%203/%345%36 => /1+2+3/456
  /1%2F2%3F3/%3F%35%20 => /1%2F2?3/%3F5+

If we don't do something like this every user need to do URL-decoding on the path.

We could still add a pathRaw to get the un-decoded path.


Added Accepted label.

@nex3
Copy link
Member Author

nex3 commented Dec 19, 2012

Every user still needs to do URL-decoding on the path in order to handle a potential %2F, so doing decoding of characters other than %2F doesn't really matter one way or the other.

I don't think it's correct to decode %20 to + -- it adds another case that users need to worry about decoding. I'd rather see "+" get automatically changed to " " or "%20". Decoding "%20" to " " would be more consistent with the overall philosophy of automatically decoding everything, and wouldn't cause any ambiguities.

If you do decode "%20" to "+", be sure to leave "%2B" (+) encoded.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Added this to the M5 milestone.

@sgjesse
Copy link
Contributor

sgjesse commented May 14, 2013

Some time ago we removed HttpRequest.path and now one has to use HttpRequest.uri.path. This getter provides the path in its encoded form.


Added Fixed label.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Area-Library, Library-IO labels.

@nex3 nex3 added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 14, 2014
@nex3 nex3 added this to the M5 milestone May 14, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Projects
None yet
Development

No branches or pull requests

4 participants