net/url: only record RawPath when it is needed

RawPath is a hint to the desired encoding of Path.
It is ignored when it is not a valid encoding of Path,
such as when Path has been changed but RawPath has not.
It is not ignored but also not useful when it matches
the url package's natural choice of encoding.
In this latter case, set it to the empty string.
This should help drive home the point that clients
cannot in general depend on it being present and
that they should use the EncodedPath method instead.

This also reduces the impact of the change on tests,
especially tests that use reflect.DeepEqual on parsed URLs.

Change-Id: I437c51a33b85439a31c307caf1436118508ea196
Reviewed-on: https://go-review.googlesource.com/11760
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go
index 492d6d2aedf89e661de3cb06237185206165471f..bda22c88eebf083d2f750f03d604a3686c70c37c 100644 (file)
--- a/src/net/http/readrequest_test.go
+++ b/src/net/http/readrequest_test.go
@@ -45,10 +45,9 @@ var reqTests = []reqTest{
                &Request{
                        Method: "GET",
                        URL: &url.URL{
-                               Scheme:  "http",
-                               Host:    "www.techcrunch.com",
-                               Path:    "/",
-                               RawPath: "/",
+                               Scheme: "http",
+                               Host:   "www.techcrunch.com",
+                               Path:   "/",
                        },
                        Proto:      "HTTP/1.1",
                        ProtoMajor: 1,
@@ -83,8 +82,7 @@ var reqTests = []reqTest{
                &Request{
                        Method: "GET",
                        URL: &url.URL{
-                               Path:    "/",
-                               RawPath: "/",
+                               Path: "/",
                        },
                        Proto:         "HTTP/1.1",
                        ProtoMajor:    1,
@@ -110,8 +108,7 @@ var reqTests = []reqTest{
                &Request{
                        Method: "GET",
                        URL: &url.URL{
-                               Path:    "//user@host/is/actually/a/path/",
-                               RawPath: "//user@host/is/actually/a/path/",
+                               Path: "//user@host/is/actually/a/path/",
                        },
                        Proto:         "HTTP/1.1",
                        ProtoMajor:    1,
@@ -161,8 +158,7 @@ var reqTests = []reqTest{
                &Request{
                        Method: "POST",
                        URL: &url.URL{
-                               Path:    "/",
-                               RawPath: "/",
+                               Path: "/",
                        },
                        TransferEncoding: []string{"chunked"},
                        Proto:            "HTTP/1.1",
@@ -236,8 +232,7 @@ var reqTests = []reqTest{
                &Request{
                        Method: "CONNECT",
                        URL: &url.URL{
-                               Path:    "/_goRPC_",
-                               RawPath: "/_goRPC_",
+                               Path: "/_goRPC_",
                        },
                        Proto:         "HTTP/1.1",
                        ProtoMajor:    1,
@@ -308,8 +303,7 @@ var reqTests = []reqTest{
                &Request{
                        Method: "GET",
                        URL: &url.URL{
-                               Path:    "/",
-                               RawPath: "/",
+                               Path: "/",
                        },
                        Header: Header{
                                // This wasn't removed from Go 1.0 to

diff --git a/src/net/url/url.go b/src/net/url/url.go
index 2dc5e3036551b872533eaf5daebd3de9859b29b7..abcd23bb76f29ddeab33b6f3e0ce1e7fcf79cd7f 100644 (file)
--- a/src/net/url/url.go
+++ b/src/net/url/url.go
@@ -426,10 +426,16 @@ func parse(rawurl string, viaRequest bool) (url *URL, err error) {
                        goto Error
                }
        }
-       url.RawPath = rest
        if url.Path, err = unescape(rest, encodePath); err != nil {
                goto Error
        }
+       // RawPath is a hint as to the encoding of Path to use
+       // in url.EncodedPath. If that method already gets the
+       // right answer without RawPath, leave it empty.
+       // This will help make sure that people don't rely on it in general.
+       if url.EscapedPath() != rest && validEncodedPath(rest) {
+               url.RawPath = rest
+       }
        return url, nil
 
 Error:
@@ -544,7 +550,7 @@ func (u *URL) EscapedPath() string {
 }
 
 // validEncodedPath reports whether s is a valid encoded path.
-// It must contain any bytes that require escaping during path encoding.
+// 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) {

diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go
index a85014690d9dc021007daaad6eed0008d83eb2a1..dbdba67834e2f32bf8e4bfaf65fc459968b445b5 100644 (file)
--- a/src/net/url/url_test.go
+++ b/src/net/url/url_test.go
@@ -99,7 +99,6 @@ var urltests = []URLTest{
                        Scheme:   "http",
                        Host:     "www.google.com",
                        Path:     "/a b",
-                       RawPath:  "/a%20b",
                        RawQuery: "q=c+d",
                },
                "",
@@ -394,8 +393,8 @@ func ufmt(u *URL) string {
                        pass = p
                }
        }
-       return fmt.Sprintf("opaque=%q, scheme=%q, user=%#v, pass=%#v, host=%q, path=%q, rawq=%q, frag=%q",
-               u.Opaque, u.Scheme, user, pass, u.Host, u.Path, u.RawQuery, u.Fragment)
+       return fmt.Sprintf("opaque=%q, scheme=%q, user=%#v, pass=%#v, host=%q, path=%q, rawpath=%q, rawq=%q, frag=%q",
+               u.Opaque, u.Scheme, user, pass, u.Host, u.Path, u.RawPath, u.RawQuery, u.Fragment)
 }
 
 func DoTest(t *testing.T, parse func(string) (*URL, error), name string, tests []URLTest) {
@@ -405,9 +404,6 @@ func DoTest(t *testing.T, parse func(string) (*URL, error), name string, tests [
                        t.Errorf("%s(%q) returned error %s", name, tt.in, err)
                        continue
                }
-               if tt.out.RawPath == "" {
-                       tt.out.RawPath = tt.out.Path
-               }
                if !reflect.DeepEqual(u, tt.out) {
                        t.Errorf("%s(%q):\n\thave %v\n\twant %v\n",
                                name, tt.in, ufmt(u), ufmt(tt.out))