net/url: allow all valid host chars in RawPath

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 <adg@golang.org>

diff --git a/src/net/url/url.go b/src/net/url/url.go
index efbb4c36e9bf0419619d4dfdffc26d578a8a0f9a..8ffad663d5cf062e96055131a48a5c51cab66434 100644 (file)
--- a/src/net/url/url.go
+++ b/src/net/url/url.go
@@ -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

diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go
index 80a2b80efad32e80994f54a2fa1a05db9750c930..ff6e9e4541a7b6b524f74e02808bedfc5f464406 100644 (file)
--- a/src/net/url/url_test.go
+++ b/src/net/url/url_test.go
@@ -392,7 +392,7 @@ var urltests = []URLTest{
                },
                "",
        },
-       // worst case host
+       // worst case host, still round trips
        {
                "scheme://!$&'()*+,;=hello!:port/path",
                &URL{
@@ -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