net/http: make ServeMux preserve query string during redirects

Ensure that the implicitly created redirect
for
  "/route"
after
  "/route/"
has been registered doesn't lose the query string information.

Fixes #17841.

Change-Id: Ib7df9242fab8c9368a18fc0da678003d6bec63b8
Reviewed-on: https://go-review.googlesource.com/43779
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

2 files changed, 51 insertions, 0 deletions

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 5b384190b2..1d541a8e46 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -5540,6 +5540,48 @@ func TestServerValidatesMethod(t *testing.T) {
 	}
 }
 
+// Test that the special cased "/route" redirect
+// implicitly created by a registered "/route/"
+// properly sets the query string in the redirect URL.
+// See Issue 17841.
+func TestServeWithSlashRedirectKeepsQueryString(t *testing.T) {
+	setParallel(t)
+	defer afterTest(t)
+
+	writeBackQuery := func(w ResponseWriter, r *Request) {
+		fmt.Fprintf(w, "%s", r.URL.RawQuery)
+	}
+
+	mux := NewServeMux()
+	mux.HandleFunc("/testOne", writeBackQuery)
+	mux.HandleFunc("/testTwo/", writeBackQuery)
+
+	ts := httptest.NewServer(mux)
+	defer ts.Close()
+
+	tests := [...]struct {
+		path string
+		want string
+	}{
+		0: {"/testOne?this=that", "this=that"},
+		1: {"/testTwo?foo=bar", "foo=bar"},
+		2: {"/testTwo?a=1&b=2&a=3", "a=1&b=2&a=3"},
+		3: {"/testTwo?", ""},
+	}
+
+	for i, tt := range tests {
+		res, err := ts.Client().Get(ts.URL + tt.path)
+		if err != nil {
+			continue
+		}
+		slurp, _ := ioutil.ReadAll(res.Body)
+		res.Body.Close()
+		if got, want := string(slurp), tt.want; got != want {
+			t.Errorf("#%d: got = %q; want = %q", i, got, want)
+		}
+	}
+}
+
 func BenchmarkResponseStatusLine(b *testing.B) {
 	b.ReportAllocs()
 	b.RunParallel(func(pb *testing.PB) {
diff --git a/src/net/http/server.go b/src/net/http/server.go
index 45f8e1b16a..71f46a74f9 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1963,6 +1963,7 @@ func StripPrefix(prefix string, h Handler) Handler {
 // The provided code should be in the 3xx range and is usually
 // StatusMovedPermanently, StatusFound or StatusSeeOther.
 func Redirect(w ResponseWriter, r *Request, urlStr string, code int) {
+	queryAlreadySet := false
 	if u, err := url.Parse(urlStr); err == nil {
 		// If url was relative, make absolute by
 		// combining with request path.
@@ -2005,9 +2006,17 @@ func Redirect(w ResponseWriter, r *Request, urlStr string, code int) {
 				urlStr += "/"
 			}
 			urlStr += query
+			queryAlreadySet = len(query) != 0
 		}
 	}
 
+	// We should make sure not to lose the query string of
+	// the original request when doing a redirect, if not already set.
+	// See Issue 17841.
+	if !queryAlreadySet && len(r.URL.RawQuery) != 0 {
+		urlStr += "?" + r.URL.RawQuery
+	}
+
 	w.Header().Set("Location", hexEscapeNonASCII(urlStr))
 	w.WriteHeader(code)