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/http: NewRequest modifies path #8767

Closed
gopherbot opened this issue Sep 18, 2014 · 4 comments
Closed

net/http: NewRequest modifies path #8767

gopherbot opened this issue Sep 18, 2014 · 4 comments
Milestone

Comments

@gopherbot
Copy link

by stuart.carnie:

What does 'go version' print?
go version go1.3.1 darwin/amd64

What steps reproduce the problem?

Unable to fetch the following image URL when using net/http.Get or manually constructing
a http.NewRequest.  
https://d2mckvlpm046l3.cloudfront.net/787968d58b958e4aea105ad017f9a1abb6cce99c/http:%2F%2Fd.pr%2Fi%2F4EN8%2B.


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

What happened?
Server responds with a 404 Not Found

What should have happened instead?
Image data should have been returned in the response Body, such as when using a browser
or CLI tools like wget or curl
@gopherbot
Copy link
Author

Comment 1 by stuart.carnie:

URL as posted is confirmed to load in Chrome

@josharian
Copy link
Contributor

Comment 2:

Thanks for the report.
Diagnosis: http.NewRequest parses the URL, which unescapes the :, /, and + in the path
-- see http://play.golang.org/p/HNK6Qa4RfH. curl, browsers, etc. send the original path.
cloudfront doesn't treat the unescaped and the escaped paths the same, which looks like
a violation of the RFC. On the other hand, it seems like the http client shouldn't
modify the path provided to it. Brad?

Labels changed: added repo-main.

Owner changed to @bradfitz.

@bradfitz
Copy link
Contributor

Comment 3:

This is the dozenth bug in the same theme. It's been like this for ages.
The http client ultimately uses net/url.Parse which unescapes the path.
The workaround has been documented at http://golang.org/pkg/net/url/#URL but not on
url.Parse, nor on http.Get and friends.
Perhaps we need a new url.ParseRaw or something which is documented to guarantee to
round-trip (String->Parse->String), parsing into the url.Opaque field as needed to
preserve the guarantee.
Then we can adjust net/http to use url.ParseRaw instead.
Assigning to Russ for his opinion, and he can assign back to me to implement if desired.

Owner changed to @rsc.

@bradfitz
Copy link
Contributor

Comment 4:

Update: I added an example at https://golang.org/cl/171620043

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015
@rsc rsc removed the repo-main label Apr 14, 2015
@rsc rsc closed this as completed in 874a605 Jun 22, 2015
@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe Jun 22, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc removed their assignment Jun 23, 2022
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

5 participants