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: RequestURI encoded path should not encode '!' #6784

Closed
ghost opened this issue Nov 18, 2013 · 17 comments
Closed

net/url: RequestURI encoded path should not encode '!' #6784

ghost opened this issue Nov 18, 2013 · 17 comments

Comments

@ghost
Copy link

ghost commented Nov 18, 2013

see https://groups.google.com/forum/#!topic/golang-nuts/5er6Ud_V0-U
@ghost
Copy link
Author

ghost commented Nov 18, 2013

Comment 1:

see http://play.golang.org/p/X6LGcNbHzA, it's more obvious.
it's affected by all struts2 framework, it's serious.

@davecheney
Copy link
Contributor

Comment 2:

Here is a smaller reproduction,
http://play.golang.org/p/xPQ61lbUqE
The bone of contention is the encoding of !. I am not sure if this is a problem or not.

@ghost
Copy link
Author

ghost commented Nov 18, 2013

Comment 3:

The https://www.shipin7.com/user/userAction%21goRegister.action page is incorrect.
https://www.shipin7.com/user/userAction!goRegister.action page is ok.
you can compare above in Browser.
http.Client.Do(), http.Get() internal encode '!' and send whole Request to server.
I think '!' should not encode because of RFC3986

@gopherbot
Copy link

Comment 4 by hongruiqi:

In RFC 2396:
      reserved    = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" |
                    "$" | ","
      unreserved  = alphanum | mark
      mark        = "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"
In RFC 3986:
      reserved    = gen-delims / sub-delims
      gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"
      sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="
      unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
https://code.google.com/p/go/source/detail?r=6b46fb967ca4a48caf486f4452c4358251f91aad
The CL above only removes !*\() from unreserved part(the \ may be wrong, it should be
"'"), 
but doesn't add []!'()* to the reserved part. So I think it's a bug.

@gopherbot
Copy link

Comment 5 by hongruiqi:

sorry, I mistake '\'' as '\', nothing wrong here.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 6:

Labels changed: added go1.3maybe.

@dsymonds
Copy link
Contributor

dsymonds commented Dec 2, 2013

Comment 7:

Labels changed: added priority-soon, packagebug, removed priority-triage.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 8:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 9:

Labels changed: added repo-main.

@gopherbot
Copy link

Comment 10:

This also triggers with the hashbang style single-page web app url fragments:
http://play.golang.org/p/-kx5yULrzl
    u, err := url.Parse("http://foo.bar/#!quux")
    // http://foo.bar/#%21quux
See https://developers.google.com/webmasters/ajax-crawling/ for more.

@minux
Copy link
Member

minux commented Dec 5, 2013

Comment 11:

https://golang.org/cl/31400043/
Does anyone know why ! is left out in the first place? was that intentional?

Status changed to Started.

@andybalholm
Copy link
Contributor

Comment 12:

Some examples of URLs where parentheses don't work if they're escaped:
http://web.signaltiretrader.com/(S(5iexcz551ptpgo45g03mgz45))/Themes/css/ploneColumns.css
and the LinkedIn API URLs discussed at
https://groups.google.com/forum/#!searchin/golang-nuts/url$20escaping/golang-nuts/Mro8TGrb3y8/eW8QCx_iFYMJ

@gopherbot
Copy link

Comment 13:

jkbbwr on IRC pointed out that slashes in queries get quoted too:
http://play.golang.org/p/EiRhkOT8im
Relevant RFC: http://tools.ietf.org/html/rfc3986#section-3.4

@gopherbot
Copy link

Comment 14 by hongruiqi:

Slashes not quoted in queries may causes some server failed to handle?

@andybalholm
Copy link
Contributor

Comment 15:

This appears to be a duplicate of 5684.

@andybalholm
Copy link
Contributor

@bradfitz
Copy link
Contributor

Comment 17:

Status changed to Duplicate.

Merged into issue #5684.

@ghost ghost added duplicate labels Jan 15, 2014
@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

7 participants