Skip to content

Commit

Permalink
net/http: support URLs without schemes in http.Redirect
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
honsiorovskyi authored and bradfitz committed Jan 12, 2016
1 parent 27df2e3 commit eca3618
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
35 changes: 35 additions & 0 deletions src/net/http/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions src/net/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit eca3618

Please sign in to comment.