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

net/url: url.Parse decodes the Path portion of a URL #8248

Closed
gopherbot opened this issue Jun 20, 2014 · 4 comments
Closed

net/url: url.Parse decodes the Path portion of a URL #8248

gopherbot opened this issue Jun 20, 2014 · 4 comments

Comments

@gopherbot
Copy link
Contributor

by stuart.carnie:

What does 'go version' print?

1.3

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

http://play.golang.org/p/ZAdvTSBQm5

What happened?

The Path section of the URL has been decoded, which creates a new (and invalid).
server.com is expecting to find a file in "/images" called
"http%3A%2F%2Fdroplr.static.s3.amazonaws.com%2Fassets%2Femail%2Fimg%2Ftips-mac%2Ftips-mac-4.jpg"
not
"http://droplr.static.s3.amazonaws.com/assets/email/img/tips-mac/tips-mac-4.jpg";

What should have happened instead?
@ianlancetaylor
Copy link
Contributor

Comment 1:

This is documented behaviour: http://golang.org/pkg/net/url/#URL .

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Contributor Author

Comment 2 by stuart.carnie:

I see that in the docs, however it simply documents a significant floor of the API. As a
consumer, it renders the http.(Get|Head|Post|PostForm) and http.NewRequest APIs
unreliable for a class of URLs that use encoded values in path segments.
As a workaround, I will need to write my own url parse function and set the URL.Opaque
property in order to allow requests with encoded path value to succeed.

@gopherbot
Copy link
Contributor Author

Comment 3 by stuart.carnie:

Per RFC 3986, Section 7.3 Back-end Transcoding:
"Percent-encoded octets must be decoded at some point during the dereference process. 
Applications must split the URI into its components and subcomponents prior to decoding
the octets, as otherwise the decoded octets might be mistaken for delimiters."
http://tools.ietf.org/html/rfc3986#section-7.3
I'd interpret that as meaning the path must be separated into it's components, separated
by "/" and then decoded into an array OR if the path is retained as a string, it should
not be decoded at all

@gopherbot
Copy link
Contributor Author

Comment 4 by stuart.carnie:

I'm resurrecting this issue, because the "documented behavior" to use Opaque is broken
in other scenarios…I'll log all the data in a follow up issue

rsc added a commit that referenced this issue Jun 22, 2015
Historically we have declined to try to provide real support for URLs
that contain %2F in the path, but they seem to be popping up more
often, especially in (arguably ill-considered) REST APIs that shoehorn
entire paths into individual path elements.

The obvious thing to do is to introduce a URL.RawPath field that
records the original encoding of Path and then consult it during
URL.String and URL.RequestURI. The problem with the obvious thing
is that it breaks backward compatibility: if someone parses a URL
into u, modifies u.Path, and calls u.String, they expect the result
to use the modified u.Path and not the original raw encoding.

Split the difference by treating u.RawPath as a hint: the observation
is that there are many valid encodings of u.Path. If u.RawPath is one
of them, use it. Otherwise compute the encoding of u.Path as before.

If a client does not use RawPath, the only change will be that String
selects a different valid encoding sometimes (the original passed
to Parse).

This ensures that, for example, HTTP requests use the exact
encoding passed to http.Get (or http.NewRequest, etc).

Also add new URL.EscapedPath method for access to the actual
escaped path. Clients should use EscapedPath instead of
reading RawPath directly.

All the old workarounds remain valid.

Fixes #5777.
Might help #9859.
Fixes #7356.
Fixes #8767.
Fixes #8292.
Fixes #8450.
Fixes #4860.
Fixes #10887.
Fixes #3659.
Fixes #8248.
Fixes #6658.
Reduces need for #2782.

Change-Id: I77b88f14631883a7d74b72d1cf19b0073d4f5473
Reviewed-on: https://go-review.googlesource.com/11302
Reviewed-by: Brad Fitzpatrick <[email protected]>
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants