net/http: handle Request.URL.RawPath in StripPrefix

The StripPrefix wrapper strips a prefix string from the request's
URL.Path field, but doesn't touch the RawPath field. This leads to the
confusing situation when StripPrefix handles a request with URL.RawPath
populated (due to some escaped characters in the request path) and the
wrapped request's RawPath contains the prefix but Path does not.

This change modifies StripPrefix to strip the prefix from both Path and
RawPath. If there are escaped characters in the prefix part of the
request URL the stripped handler serves a 404 instead of invoking the
underlying handler with a mismatched Path/RawPath pair.

This is a backward incompatible change for a very small minority of
requests; I would be surprised if anyone is depending on this behavior,
but it is possible. If that's the case, we could make a more
conservative change where the RawPath is trimmed if possible, but when
the prefix contains escaped characters then we don't 404 but rather send
through the invalid Path/RawPath pair as before.

Fixes #24366

Change-Id: I7030b8c183a3dfce307bc0272bba9a18df4cfe08
Reviewed-on: https://go-review.googlesource.com/c/go/+/233637
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

diff --git a/doc/go1.16.html b/doc/go1.16.html
index 4753cf914d5370ec75da03e7400903377f7f5b6b..09e974d07c26b3ddf5f97e8aa485d3b36270fa54 100644 (file)
--- a/doc/go1.16.html
+++ b/doc/go1.16.html
@@ -112,3 +112,17 @@ Do not send CLs removing the interior tags from such phrases.
 <p>
   TODO
 </p>
+
+<p>
+  In the <a href="/pkg/net/http/"><code>net/http</code></a> package, the
+  behavior of <a href="/pkg/net/http/#StripPrefix"><code>StripPrefix</code></a>
+  has been changed to strip the prefix from the request URL's
+  <code>RawPath</code> field in addition to its <code>Path</code> field.
+  In past releases, only the <code>Path</code> field was trimmed, and so if the
+  request URL contained any escaped characters the URL would be modified to
+  have mismatched <code>Path</code> and <code>RawPath</code> fields.
+  In Go 1.16, <code>StripPrefix</code> trims both fields.
+  If there are escaped characters in the prefix part of the request URL the
+  handler serves a 404 instead of its previous behavior of invoking the
+  underlying handler with a mismatched <code>Path</code>/<code>RawPath</code> pair.
+</p>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 5f5693277828b3d30e7c7e56dc08b6394ef2f6f7..635bf5dfc910b3413b9725bdc0c9cf79e02f7317 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -2849,29 +2849,47 @@ func TestStripPrefix(t *testing.T) {
        defer afterTest(t)
        h := HandlerFunc(func(w ResponseWriter, r *Request) {
                w.Header().Set("X-Path", r.URL.Path)
+               w.Header().Set("X-RawPath", r.URL.RawPath)
        })
-       ts := httptest.NewServer(StripPrefix("/foo", h))
+       ts := httptest.NewServer(StripPrefix("/foo/bar", h))
        defer ts.Close()
 
        c := ts.Client()
 
-       res, err := c.Get(ts.URL + "/foo/bar")
-       if err != nil {
-               t.Fatal(err)
-       }
-       if g, e := res.Header.Get("X-Path"), "/bar"; g != e {
-               t.Errorf("test 1: got %s, want %s", g, e)
-       }
-       res.Body.Close()
-
-       res, err = Get(ts.URL + "/bar")
-       if err != nil {
-               t.Fatal(err)
-       }
-       if g, e := res.StatusCode, 404; g != e {
-               t.Errorf("test 2: got status %v, want %v", g, e)
+       cases := []struct {
+               reqPath string
+               path    string // If empty we want a 404.
+               rawPath string
+       }{
+               {"/foo/bar/qux", "/qux", ""},
+               {"/foo/bar%2Fqux", "/qux", "%2Fqux"},
+               {"/foo%2Fbar/qux", "", ""}, // Escaped prefix does not match.
+               {"/bar", "", ""},           // No prefix match.
+       }
+       for _, tc := range cases {
+               t.Run(tc.reqPath, func(t *testing.T) {
+                       res, err := c.Get(ts.URL + tc.reqPath)
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+                       res.Body.Close()
+                       if tc.path == "" {
+                               if res.StatusCode != StatusNotFound {
+                                       t.Errorf("got %q, want 404 Not Found", res.Status)
+                               }
+                               return
+                       }
+                       if res.StatusCode != StatusOK {
+                               t.Fatalf("got %q, want 200 OK", res.Status)
+                       }
+                       if g, w := res.Header.Get("X-Path"), tc.path; g != w {
+                               t.Errorf("got Path %q, want %q", g, w)
+                       }
+                       if g, w := res.Header.Get("X-RawPath"), tc.rawPath; g != w {
+                               t.Errorf("got RawPath %q, want %q", g, w)
+                       }
+               })
        }
-       res.Body.Close()
 }
 
 // https://golang.org/issue/18952.

diff --git a/src/net/http/server.go b/src/net/http/server.go
index d41b5f6f487953d187627fd749dc9385699bbd6f..ed5de350a99a294930d0d07d367ba5b15ccfdb38 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2062,22 +2062,26 @@ func NotFound(w ResponseWriter, r *Request) { Error(w, "404 page not found", Sta
 // that replies to each request with a ``404 page not found'' reply.
 func NotFoundHandler() Handler { return HandlerFunc(NotFound) }
 
-// StripPrefix returns a handler that serves HTTP requests
-// by removing the given prefix from the request URL's Path
-// and invoking the handler h. StripPrefix handles a
-// request for a path that doesn't begin with prefix by
-// replying with an HTTP 404 not found error.
+// StripPrefix returns a handler that serves HTTP requests by removing the
+// given prefix from the request URL's Path (and RawPath if set) and invoking
+// the handler h. StripPrefix handles a request for a path that doesn't begin
+// with prefix by replying with an HTTP 404 not found error. The prefix must
+// match exactly: if the prefix in the request contains escaped characters
+// the reply is also an HTTP 404 not found error.
 func StripPrefix(prefix string, h Handler) Handler {
        if prefix == "" {
                return h
        }
        return HandlerFunc(func(w ResponseWriter, r *Request) {
-               if p := strings.TrimPrefix(r.URL.Path, prefix); len(p) < len(r.URL.Path) {
+               p := strings.TrimPrefix(r.URL.Path, prefix)
+               rp := strings.TrimPrefix(r.URL.RawPath, prefix)
+               if len(p) < len(r.URL.Path) && (r.URL.RawPath == "" || len(rp) < len(r.URL.RawPath)) {
                        r2 := new(Request)
                        *r2 = *r
                        r2.URL = new(url.URL)
                        *r2.URL = *r.URL
                        r2.URL.Path = p
+                       r2.URL.RawPath = rp
                        h.ServeHTTP(w, r2)
                } else {
                        NotFound(w, r)