From 674ef1c28022f33fa7200f59db45e604557f0380 Mon Sep 17 00:00:00 2001 From: Kush Mansingh Date: Wed, 24 Aug 2016 09:45:17 -0400 Subject: [PATCH] Add mechanism to route based on the escaped path (#184) * Add mechanism to route based on the escaped path, correct request mocking in tests * Remove unneccessary regex matching, substitute with string slicing * Add test case and handling for requests with no host/scheme --- mux.go | 30 ++++++++++++++++++++++++++++- mux_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++- regexp.go | 11 ++++++----- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/mux.go b/mux.go index 5e0dd69..514bfb0 100644 --- a/mux.go +++ b/mux.go @@ -10,6 +10,7 @@ import ( "net/http" "path" "regexp" + "strings" ) // NewRouter returns a new router instance. @@ -76,8 +77,9 @@ 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 := getPath(req) // Clean path to canonical form and redirect. - if p := cleanPath(req.URL.Path); p != req.URL.Path { + if p := cleanPath(path); p != path { // 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: @@ -358,6 +360,32 @@ func setCurrentRoute(r *http.Request, val interface{}) *http.Request { // Helpers // ---------------------------------------------------------------------------- +// getPath returns the escaped path if possible; doing what URL.EscapedPath() +// which was added in go1.5 does +func getPath(req *http.Request) string { + if req.RequestURI != "" { + // Extract the path from RequestURI (which is escaped unlike URL.Path) + // as detailed here as detailed in https://golang.org/pkg/net/url/#URL + // for < 1.5 server side workaround + // http://localhost/path/here?v=1 -> /path/here + path := req.RequestURI + if i := len(req.URL.Scheme); i > 0 { + path = path[i+len(`://`):] + } + if i := len(req.URL.Host); i > 0 { + path = path[i:] + } + if i := strings.LastIndex(path, "?"); i > -1 { + path = path[:i] + } + if i := strings.LastIndex(path, "#"); i > -1 { + path = path[:i] + } + return path + } + return req.URL.Path +} + // cleanPath returns the canonical path for p, eliminating . and .. elements. // Borrowed from the net/http package. func cleanPath(p string) string { diff --git a/mux_test.go b/mux_test.go index e8e2005..1670764 100644 --- a/mux_test.go +++ b/mux_test.go @@ -5,6 +5,8 @@ package mux import ( + "bufio" + "bytes" "errors" "fmt" "net/http" @@ -280,6 +282,16 @@ func TestPath(t *testing.T) { pathTemplate: `/111`, shouldMatch: false, }, + { + title: "Path route, match root with no host", + route: new(Route).Path("/"), + request: newRequest("GET", "/"), + vars: map[string]string{}, + host: "", + path: "/", + pathTemplate: `/`, + shouldMatch: true, + }, { title: "Path route, wrong path in request in request URL", route: new(Route).Path("/111/222/333"), @@ -309,6 +321,16 @@ func TestPath(t *testing.T) { pathTemplate: `/111/{v1:[0-9]{3}}/333`, shouldMatch: false, }, + { + title: "Path route, URL with encoded slash does match", + route: new(Route).Path("/v1/{v1}/v2"), + request: newRequest("GET", "http://localhost/v1/1%2F2/v2"), + vars: map[string]string{"v1": "1%2F2"}, + host: "", + path: "/v1/1%2F2/v2", + pathTemplate: `/v1/{v1}/v2`, + shouldMatch: true, + }, { title: "Path route with multiple patterns, match", route: new(Route).Path("/{v1:[0-9]{3}}/{v2:[0-9]{3}}/{v3:[0-9]{3}}"), @@ -1466,11 +1488,42 @@ func stringMapEqual(m1, m2 map[string]string) bool { return true } -// newRequest is a helper function to create a new request with a method and url +// newRequest is a helper function to create a new request with a method and url. +// The request returned is a 'server' request as opposed to a 'client' one through +// simulated write onto the wire and read off of the wire. +// The differences between requests are detailed in the net/http package. func newRequest(method, url string) *http.Request { req, err := http.NewRequest(method, url, nil) if err != nil { panic(err) } + // extract the escaped original host+path from url + // http://localhost/path/here?v=1#frag -> //localhost/path/here + opaque := "" + if i := len(req.URL.Scheme); i > 0 { + opaque = url[i+1:] + } + + if i := strings.LastIndex(opaque, "?"); i > -1 { + opaque = opaque[:i] + } + if i := strings.LastIndex(opaque, "#"); i > -1 { + opaque = opaque[:i] + } + + // Escaped host+path workaround as detailed in https://golang.org/pkg/net/url/#URL + // for < 1.5 client side workaround + req.URL.Opaque = opaque + + // Simulate writing to wire + var buff bytes.Buffer + req.Write(&buff) + ioreader := bufio.NewReader(&buff) + + // Parse request off of 'wire' + req, err = http.ReadRequest(ioreader) + if err != nil { + panic(err) + } return req } diff --git a/regexp.go b/regexp.go index 08710bc..99d41a8 100644 --- a/regexp.go +++ b/regexp.go @@ -149,8 +149,8 @@ func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool { if r.matchQuery { return r.matchQueryString(req) } - - return r.regexp.MatchString(req.URL.Path) + path := getPath(req) + return r.regexp.MatchString(path) } return r.regexp.MatchString(getHost(req)) @@ -253,14 +253,15 @@ func (v *routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route) extractVars(host, matches, v.host.varsN, m.Vars) } } + path := getPath(req) // Store path variables. if v.path != nil { - matches := v.path.regexp.FindStringSubmatchIndex(req.URL.Path) + matches := v.path.regexp.FindStringSubmatchIndex(path) if len(matches) > 0 { - extractVars(req.URL.Path, matches, v.path.varsN, m.Vars) + extractVars(path, matches, v.path.varsN, m.Vars) // Check if we should redirect. if v.path.strictSlash { - p1 := strings.HasSuffix(req.URL.Path, "/") + p1 := strings.HasSuffix(path, "/") p2 := strings.HasSuffix(v.path.template, "/") if p1 != p2 { u, _ := url.Parse(req.URL.String())