net/http: preserve path escaping in redirects

Currently, when performing path sanitization and trailing-slash
redirects, we do not populate the RawPath field of the returned URL. As
a result, when a URL path is already escaped, our redirect behavior can
be rather surprising:

- When "/a//b%2fc" is sanitized, it is double-escaped, becoming
  "/a/b/%252fc".
- When doing trailing slash-redirect, "/a%2fb" will lose its escaping,
  becoming "/a/b/" instead of "/a%2fb/".

Fix this by populating the RawPath field.

Also updated TestServeWithSlashRedirectForHostPatterns that still uses
the old 301 status code rather than 307 after CL 720820.

Fixes #79897

Change-Id: Ibdd1f119afdd6824e52185be3852234a6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/792860
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>

src/net/http/serve_test.go

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 7e83984..f8710af 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -308,6 +308,7 @@
 	{"codesearch.google.com/search", serve(202)},
 	{"codesearch.google.com/", serve(203)},
 	{"example.com/", HandlerFunc(checkQueryStringHandler)},
+	{"/pkg/bar/extra%2fpath", serve(200)},
 }
 
 // serve returns a handler that sends a response with the given code.
@@ -427,6 +428,9 @@
 	{"GET", "google.com", "/", 404, false},
 	{"GET", "example.com", "/test/?example.com/test/", 200, false},
 	{"GET", "example.com", "test/?example.com/test/", 200, true},
+	{"GET", "google.com", "/pkg/bar//extra%2fpath", 200, true},
+	{"GET", "google.com", "/dir/b%2fc/..", 200, true},
+	{"GET", "google.com", "/doesnotexist/b%2fc/..", 404, true},
 }
 
 // TestServeMuxHandlerRedirects tests that automatic redirects generated by
@@ -601,6 +605,7 @@
 	mux.Handle("example.com:3000/pkg/connect/", stringHandler("example.com:3000/pkg/connect/"))
 	mux.Handle("example.com:9000/", stringHandler("example.com:9000/"))
 	mux.Handle("/pkg/baz/", stringHandler("/pkg/baz/"))
+	mux.Handle("example.com/a%2fb/", stringHandler("example.com/a%2fb/"))
 
 	tests := []struct {
 		method string
@@ -622,6 +627,7 @@
 		{"CONNECT", "http://example.com:3000/pkg/foo", 404, "", ""},
 		{"CONNECT", "http://example.com:3000/pkg/baz", 307, "/pkg/baz/", ""},
 		{"CONNECT", "http://example.com:3000/pkg/connect", 307, "/pkg/connect/", ""},
+		{"GET", "http://example.com/a%2fb", 307, "/a%2fb/", ""},
 	}
 
 	for i, tt := range tests {
@@ -633,7 +639,7 @@
 			t.Errorf("#%d: Status = %d; want = %d", i, got, want)
 		}
 
-		if tt.code == 301 {
+		if tt.code == 307 {
 			if got, want := w.HeaderMap.Get("Location"), tt.loc; got != want {
 				t.Errorf("#%d: Location = %q; want = %q", i, got, want)
 			}

src/net/http/server.go

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 933dcc9..1ef5621 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2747,7 +2747,7 @@
 			if n != nil {
 				patStr = n.pattern.String()
 			}
-			u := &url.URL{Path: path, RawQuery: r.URL.RawQuery}
+			u := urlFromEscaped(path, r.URL.RawQuery)
 			return RedirectHandler(u.String(), StatusTemporaryRedirect), patStr, nil, nil
 		}
 	}
@@ -2792,12 +2792,35 @@
 			// of findHandler, and that method returns before it does the "n == nil" check where
 			// the first return value matters. We return it here only to make the pattern available
 			// to findHandler.
-			return n2, nil, &url.URL{Path: cleanPath(u.Path) + "/", RawQuery: u.RawQuery}
+			return n2, nil, urlFromEscaped(path, u.RawQuery)
 		}
 	}
 	return n, matches, nil
 }
 
+// urlFromEscaped returns a url.URL constructed from an escaped path and a raw
+// query.
+//
+// It ensures that the Path and RawPath fields are in sync by unescaping the
+// escaped path. Populating only the Path field and leaving RawPath empty (or
+// failing to keep them in sync) can cause url.URL.String to produce a URL with
+// either unexpected escaping (e.g., double-escaping "%" into "%25" in an
+// already escaped path) or a lack thereof (e.g., losing the escaping of "%2f"
+// and turning it into a literal path separator "/").
+func urlFromEscaped(escaped, rawQuery string) *url.URL {
+	unescaped, err := url.PathUnescape(escaped)
+	// Should be impossible, since ServeMux will reject unparsable URLs way
+	// earlier.
+	if err != nil {
+		unescaped = escaped
+	}
+	return &url.URL{
+		Path:     unescaped,
+		RawPath:  escaped,
+		RawQuery: rawQuery,
+	}
+}
+
 // exactMatch reports whether the node's pattern exactly matches the path.
 // As a special case, if the node is nil, exactMatch return false.
 //