[go] net/http: use relative path in Location redirect

Roland Shoemaker (Gerrit)

unread,

Apr 26, 2021, 11:17:06 PM4/26/21

to Katie Hockman, Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Katie Hockman, Filippo Valsorda.

Roland Shoemaker would like Katie Hockman and Filippo Valsorda to review this change.

net/http: use relative path in Location redirect

If the cleaned path did not match the requested path, ServeMux.Handler
would return a Location header which reflected the hostname in the
request, possibly leading to an incorrect redirect. Instead the
Location header should be relative, like the other cases in
ServeMux.Handler.

Change-Id: I2c220d925e708061bc128f0bdc96cca7a32764d3
---
M src/net/http/serve_test.go
M src/net/http/server.go
2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index f868741..3c1464c 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -6507,3 +6507,22 @@
 		t.Fatalf("unexpected value read from body:\ngot: %q\nwant: %q", b, "hello")
 	}
 }
+
+func TestMuxRedirectRelative(t *testing.T) {
+	setParallel(t)
+	req, err := ReadRequest(bufio.NewReader(strings.NewReader("GET http://example.com HTTP/1.1\r\nHost: test\r\n\r\n")))
+	if err != nil {
+		t.Errorf("%s", err)
+	}
+	mux := NewServeMux()
+	resp := httptest.NewRecorder()
+	mux.ServeHTTP(resp, req)
+	if loc, expected := resp.Header().Get("Location"), "/"; loc != expected {
+		t.Errorf("Expected Location header set to %q; got %q", expected, loc)
+		return
+	}
+	if code, expected := resp.Code, StatusMovedPermanently; code != expected {
+		t.Errorf("Expected response code of StatusMovedPermanently; got %d", code)
+		return
+	}
+}
diff --git a/src/net/http/server.go b/src/net/http/server.go
index e52a78e..4e73508 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2404,9 +2404,8 @@
 
 	if path != r.URL.Path {
 		_, pattern = mux.handler(host, path)
-		url := *r.URL
-		url.Path = path
-		return RedirectHandler(url.String(), StatusMovedPermanently), pattern
+		u := &url.URL{Path: path, RawQuery: r.URL.RawQuery}
+		return RedirectHandler(u.String(), StatusMovedPermanently), pattern
 	}
 
 	return mux.handler(host, r.URL.Path)

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Katie Hockman (Gerrit)

unread,

Apr 27, 2021, 5:08:14 PM4/27/21

to Roland Shoemaker, goph...@pubsubhelper.golang.org, Go Bot, Filippo Valsorda, golang-co...@googlegroups.com

Attention is currently required from: Roland Shoemaker, Filippo Valsorda.

Patch set 1:Run-TryBot +1Code-Review +2Trust +1

View Change

1 comment:

File src/net/http/serve_test.go:
- Patch Set #1, Line 6520: if loc, expected := resp.Header().Get("Location"), "/"; loc != expected {
  nit: common pattern in general, and in this file, is to use "got, want". e.g.
```
    if got, want := resp.Header().Get("Location"), "/"; got != want {
        t.Errorf("Location header expected %q; got %q", got, want)
	return
    }
```

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Roland Shoemaker (Gerrit)

unread,

Apr 28, 2021, 12:14:42 PM4/28/21

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Roland Shoemaker, Filippo Valsorda.

Roland Shoemaker uploaded patch set #2 to this change.

View Change

net/http: use relative path in Location redirect

If the cleaned path did not match the requested path, ServeMux.Handler
would return a Location header which reflected the hostname in the
request, possibly leading to an incorrect redirect. Instead the
Location header should be relative, like the other cases in
ServeMux.Handler.

Change-Id: I2c220d925e708061bc128f0bdc96cca7a32764d3
---
M src/net/http/serve_test.go
M src/net/http/server.go
2 files changed, 21 insertions(+), 3 deletions(-)

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Roland Shoemaker (Gerrit)

unread,

Apr 28, 2021, 12:15:02 PM4/28/21

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Roland Shoemaker, Filippo Valsorda.

Roland Shoemaker uploaded patch set #3 to this change.

View Change

net/http: use relative path in Location redirect

If the cleaned path did not match the requested path, ServeMux.Handler
would return a Location header which reflected the hostname in the
request, possibly leading to an incorrect redirect. Instead the
Location header should be relative, like the other cases in
ServeMux.Handler.

Change-Id: I2c220d925e708061bc128f0bdc96cca7a32764d3
---
M src/net/http/serve_test.go
M src/net/http/server.go

2 files changed, 19 insertions(+), 3 deletions(-)

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Roland Shoemaker (Gerrit)

unread,

Apr 28, 2021, 12:15:13 PM4/28/21

to goph...@pubsubhelper.golang.org, Katie Hockman, Go Bot, Filippo Valsorda, golang-co...@googlegroups.com

Attention is currently required from: Filippo Valsorda.

Patch set 3:Run-TryBot +1Trust +1

View Change

1 comment:

File src/net/http/serve_test.go:

- Patch Set #1, Line 6520: if loc, expected := resp.Header().Get("Location"), "/"; loc != expected {

- nit: common pattern in general, and in this file, is to use "got, want". e.g. […]
  Done

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Roland Shoemaker (Gerrit)

unread,

May 3, 2021, 1:34:15 PM5/3/21

to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Katie Hockman, Filippo Valsorda, golang-co...@googlegroups.com

Roland Shoemaker submitted this change.

View Change

Approvals:
  Katie Hockman: Looks good to me, approved; Trusted
  Roland Shoemaker: Trusted; Run TryBots
  Go Bot: TryBots succeeded

net/http: use relative path in Location redirect

If the cleaned path did not match the requested path, ServeMux.Handler
would return a Location header which reflected the hostname in the
request, possibly leading to an incorrect redirect. Instead the
Location header should be relative, like the other cases in
ServeMux.Handler.

Change-Id: I2c220d925e708061bc128f0bdc96cca7a32764d3

Reviewed-on: https://go-review.googlesource.com/c/go/+/313950
Trust: Roland Shoemaker <rol...@golang.org>
Trust: Katie Hockman <ka...@golang.org>
Run-TryBot: Roland Shoemaker <rol...@golang.org>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Katie Hockman <ka...@golang.org>


---
M src/net/http/serve_test.go
M src/net/http/server.go
2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index f868741..a971468 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -6507,3 +6507,20 @@


 		t.Fatalf("unexpected value read from body:\ngot: %q\nwant: %q", b, "hello")
 	}
 }
+
+func TestMuxRedirectRelative(t *testing.T) {
+	setParallel(t)
+	req, err := ReadRequest(bufio.NewReader(strings.NewReader("GET http://example.com HTTP/1.1\r\nHost: test\r\n\r\n")))
+	if err != nil {
+		t.Errorf("%s", err)
+	}
+	mux := NewServeMux()
+	resp := httptest.NewRecorder()
+	mux.ServeHTTP(resp, req)

+	if got, want := resp.Header().Get("Location"), "/"; got != want {
+		t.Errorf("Location header expected %q; got %q", want, got)
+	}
+	if got, want := resp.Code, StatusMovedPermanently; got != want {
+		t.Errorf("Expected response code %d; got %d", want, got)


+	}
+}
diff --git a/src/net/http/server.go b/src/net/http/server.go
index e52a78e..4e73508 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2404,9 +2404,8 @@
 
 	if path != r.URL.Path {
 		_, pattern = mux.handler(host, path)
-		url := *r.URL
-		url.Path = path
-		return RedirectHandler(url.String(), StatusMovedPermanently), pattern
+		u := &url.URL{Path: path, RawQuery: r.URL.RawQuery}
+		return RedirectHandler(u.String(), StatusMovedPermanently), pattern
 	}
 
 	return mux.handler(host, r.URL.Path)

1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

The name of the file: src/net/http/serve_test.go
Insertions: 4, Deletions: 6.

```
@@ -6519:6526, +6519:6524 @@
-  	if loc, expected := resp.Header().Get("Location"), "/"; loc != expected {
-  		t.Errorf("Expected Location header set to %q; got %q", expected, loc)
-  		return
-  	}
-  	if code, expected := resp.Code, StatusMovedPermanently; code != expected {
-  		t.Errorf("Expected response code of StatusMovedPermanently; got %d", code)
-  		return
+  	if got, want := resp.Header().Get("Location"), "/"; got != want {
+  		t.Errorf("Location header expected %q; got %q", want, got)
+  	}
+  	if got, want := resp.Code, StatusMovedPermanently; got != want {
+  		t.Errorf("Expected response code %d; got %d", want, got)

```

To view, visit change 313950. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward