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>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 5b384190b2ce69913c56cd838d090a0cf1a8a01a..1d541a8e46f0664b2f11f84486dc606f9123a5ee 100644 (file)
--- 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 45f8e1b16a231862a441856dacc7b0fadac89ed5..71f46a74f97720de8f803afe5afe08d8b43c1629 100644 (file)
--- 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)