Skip to content

Commit

Permalink
net/url: allow all valid host chars in RawPath
Browse files Browse the repository at this point in the history
The old code was only allowing the chars we choose not to escape.
We sometimes prefer to escape chars that do not strictly need it.
Allowing those to be used in RawPath lets people override that
preference, which is in fact the whole point of RawPath (new in Go 1.5).

While we are here, also allow [ ] in RawPath.
This is not strictly spec-compliant, but it is what modern browers
do and what at least some people expect, and the [ ] do not cause
any ambiguity (the usual reason they would be escaped, as they are
part of the RFC gen-delims class).
The argument for allowing them now instead of waiting until Go 1.6
is that this way RawPath has one fixed meaning at the time it is
introduced, that we should not need to change or expand.

Fixes #5684.

Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c
Reviewed-on: https://go-review.googlesource.com/13258
Reviewed-by: Andrew Gerrand <[email protected]>
  • Loading branch information
rsc committed Aug 6, 2015
1 parent e8be9a1 commit fced03a
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
18 changes: 16 additions & 2 deletions src/net/url/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,22 @@ func (u *URL) EscapedPath() string {
// It must not contain any bytes that require escaping during path encoding.
func validEncodedPath(s string) bool {
for i := 0; i < len(s); i++ {
if s[i] != '%' && shouldEscape(s[i], encodePath) {
return false
// RFC 3986, Appendix A.
// pchar = unreserved / pct-encoded / sub-delims / ":" / "@".
// shouldEscape is not quite compliant with the RFC,
// so we check the sub-delims ourselves and let
// shouldEscape handle the others.
switch s[i] {
case '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', ':', '@':
// ok
case '[', ']':
// ok - not specified in RFC 3986 but left alone by modern browsers
case '%':
// ok - percent encoded, will decode
default:
if shouldEscape(s[i], encodePath) {
return false
}
}
}
return true
Expand Down
24 changes: 23 additions & 1 deletion src/net/url/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ var urltests = []URLTest{
},
"",
},
// worst case host
// worst case host, still round trips
{
"scheme://!$&'()*+,;=hello!:port/path",
&URL{
Expand All @@ -402,6 +402,28 @@ var urltests = []URLTest{
},
"",
},
// worst case path, still round trips
{
"http://host/!$&'()*+,;=:@[hello]",
&URL{
Scheme: "http",
Host: "host",
Path: "/!$&'()*+,;=:@[hello]",
RawPath: "/!$&'()*+,;=:@[hello]",
},
"",
},
// golang.org/issue/5684
{
"http://example.com/oid/[order_id]",
&URL{
Scheme: "http",
Host: "example.com",
Path: "/oid/[order_id]",
RawPath: "/oid/[order_id]",
},
"",
},
}

// more useful string for debugging than fmt's struct printer
Expand Down

0 comments on commit fced03a

Please sign in to comment.