From a9d8d4df616dc570e443d82bdc6d06a94a3c4ffd Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Wed, 24 May 2017 19:36:41 -0600 Subject: [PATCH] net/http: revert CL 43779 CL 43779/commit 6a6c792eef55eded7fb3165a330ec2b239b83960 broke the builds at tip, and that CL doesn't account for cases where Redirect is directly invoked with a full URL that itself has a query string. Updates #17841 Change-Id: Idb0486bae8625e1f9e033ca4cfcd87de95bc835c Reviewed-on: https://go-review.googlesource.com/44100 Reviewed-by: Brad Fitzpatrick --- src/net/http/serve_test.go | 42 -------------------------------------- src/net/http/server.go | 9 -------- 2 files changed, 51 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 2897c15228..d862bed5a8 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -5552,48 +5552,6 @@ 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 a8d32459e0..add05c24ed 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -1963,7 +1963,6 @@ 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. @@ -2006,17 +2005,9 @@ 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) -- 2.50.0