net/http: support URLs without schemes in http.Redirect

Many browsers now support schemeless URLs in the Location headers
and also it is allowed in the draft HTTP/1.1 specification (see
http://stackoverflow.com/q/4831741#comment25926312_4831741), but
Go standard library lacks support for them.

This patch implements schemeless URLs support in http.Redirect().
Since url.Parse() correctly handles schemeless URLs, I've just added
an extra condition to verify URL's Host part in the absoulute/relative
check in the http.Redirect function.

Also I've moved oldpath variable initialization inside the block
of code where it is used.

Change-Id: Ib8a6347816a83e16576f00c4aa13224a89d610b5
Reviewed-on: https://go-review.googlesource.com/14172
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 00220b471ca04ed8a8eeb5a0a08d4de34136ca80..86f4b66389c9c9b76d445d4ca306bbac538d3562 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -1906,6 +1906,41 @@ func TestRedirectBadPath(t *testing.T) {
        }
 }
 
+// Test different URL formats and schemes
+func TestRedirectURLFormat(t *testing.T) {
+       req, _ := NewRequest("GET", "http://example.com/qux/", nil)
+
+       var tests = []struct {
+               in   string
+               want string
+       }{
+               // normal http
+               {"http://foobar.com/baz", "http://foobar.com/baz"},
+               // normal https
+               {"https://foobar.com/baz", "https://foobar.com/baz"},
+               // custom scheme
+               {"test://foobar.com/baz", "test://foobar.com/baz"},
+               // schemeless
+               {"//foobar.com/baz", "//foobar.com/baz"},
+               // relative to the root
+               {"/foobar.com/baz", "/foobar.com/baz"},
+               // relative to the current path
+               {"foobar.com/baz", "/qux/foobar.com/baz"},
+               // relative to the current path (+ going upwards)
+               {"../quux/foobar.com/baz", "/quux/foobar.com/baz"},
+               // incorrect number of slashes
+               {"///foobar.com/baz", "/foobar.com/baz"},
+       }
+
+       for _, tt := range tests {
+               rec := httptest.NewRecorder()
+               Redirect(rec, req, tt.in, 302)
+               if got := rec.Header().Get("Location"); got != tt.want {
+                       t.Errorf("Redirect(%q) generated Location header %q; want %q", tt.in, got, tt.want)
+               }
+       }
+}
+
 // TestZeroLengthPostAndResponse exercises an optimization done by the Transport:
 // when there is no body (either because the method doesn't permit a body, or an
 // explicit Content-Length of zero is present), then the transport can re-use the

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 19324d02c48e8149801116e245a5860f9a1e5227..bbaf5d2cc64ac5e051737f16d88ee22e36e73052 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1665,11 +1665,12 @@ func Redirect(w ResponseWriter, r *Request, urlStr string, code int) {
                // Because of this problem, no one pays attention
                // to the RFC; they all send back just a new path.
                // So do we.
-               oldpath := r.URL.Path
-               if oldpath == "" { // should not happen, but avoid a crash if it does
-                       oldpath = "/"
-               }
-               if u.Scheme == "" {
+               if u.Scheme == "" && u.Host == "" {
+                       oldpath := r.URL.Path
+                       if oldpath == "" { // should not happen, but avoid a crash if it does
+                               oldpath = "/"
+                       }
+
                        // no leading http://server
                        if urlStr == "" || urlStr[0] != '/' {
                                // make relative path absolute