From 0228de04b84ea631200cfab1b055a3d8287c9729 Mon Sep 17 00:00:00 2001 From: Tiernan Messmer Date: Tue, 7 Sep 2021 16:24:26 +1000 Subject: [PATCH] Fix accidental double-escaping when Router.UseEncodedPath() is enabled --- mux.go | 26 +++++++++++++++++++------- mux_test.go | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/mux.go b/mux.go index f126a602..003695d8 100644 --- a/mux.go +++ b/mux.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "path" "regexp" ) @@ -174,19 +175,30 @@ func (r *Router) Match(req *http.Request, match *RouteMatch) bool { // mux.Vars(request). func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { if !r.skipClean { - path := req.URL.Path + urlPath := req.URL.Path if r.useEncodedPath { - path = req.URL.EscapedPath() + urlPath = req.URL.EscapedPath() } // Clean path to canonical form and redirect. - if p := cleanPath(path); p != path { - + if p := cleanPath(urlPath); p != urlPath { // Added 3 lines (Philip Schlump) - It was dropping the query string and #whatever from query. // This matches with fix in go 1.2 r.c. 4 for same problem. Go Issue: // http://code.google.com/p/go/issues/detail?id=5252 - url := *req.URL - url.Path = p - p = url.String() + reqURL := *req.URL + + if r.useEncodedPath { + pURL, err := url.ParseRequestURI(p) + if err != nil { + // This shouldn't be possible, but fall back to old behaviour if some edge case triggers it + reqURL.Path = p + } else { + reqURL.Path = pURL.Path + reqURL.RawPath = pURL.RawPath + } + } else { + reqURL.Path = p + } + p = reqURL.String() w.Header().Set("Location", p) w.WriteHeader(http.StatusMovedPermanently) diff --git a/mux_test.go b/mux_test.go index 2d8d2b3e..39dc2943 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1574,6 +1574,24 @@ func TestUseEncodedPath(t *testing.T) { } } +func TestUseEncodedPathEscaping(t *testing.T) { + r := NewRouter() + r.UseEncodedPath() + + req, _ := http.NewRequest("GET", "http://localhost/foo/../bar%25", nil) + res := NewRecorder() + r.ServeHTTP(res, req) + + if len(res.HeaderMap["Location"]) != 1 { + t.Fatalf("Expected redirect from path clean") + } + + expected := "http://localhost/bar%25" + if res.HeaderMap["Location"][0] != expected { + t.Errorf("Expected redirect location to %s, found %s", expected, res.HeaderMap["Location"][0]) + } +} + func TestWalkSingleDepth(t *testing.T) { r0 := NewRouter() r1 := NewRouter()