From 9919645bd41d7673898c7831f3fd39e1f8ca42bf Mon Sep 17 00:00:00 2001 From: Nicholas Rawlings Date: Thu, 5 May 2016 00:15:01 -0400 Subject: [PATCH 1/3] Add support for percent-encoded slashes in parameter values --- router.go | 8 +++++--- router_test.go | 34 ++++++++++++++++++++-------------- tree.go | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 18 deletions(-) diff --git a/router.go b/router.go index bb173300..99ffe9f6 100644 --- a/router.go +++ b/router.go @@ -337,7 +337,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { defer r.recv(w, req) } - path := req.URL.Path + path := req.URL.EscapedPath() if root := r.trees[req.Method]; root != nil { if handle, ps, tsr := root.getValue(path); handle != nil { @@ -353,9 +353,11 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { if tsr && r.RedirectTrailingSlash { if len(path) > 1 && path[len(path)-1] == '/' { - req.URL.Path = path[:len(path)-1] + req.URL.RawPath = path[:len(path)-1] + req.URL.Path = req.URL.Path[:len(req.URL.Path)-1] } else { - req.URL.Path = path + "/" + req.URL.RawPath = path + "/" + req.URL.Path += "/" } http.Redirect(w, req, req.URL.String(), code) return diff --git a/router_test.go b/router_test.go index 88895a3a..d017ad8a 100644 --- a/router_test.go +++ b/router_test.go @@ -46,24 +46,30 @@ func TestParams(t *testing.T) { } func TestRouter(t *testing.T) { - router := New() + testNames := map[string]Params{ + "gopher": Params{Param{"name", "gopher"}}, + "go%2f1.6": Params{Param{"name", "go/1.6"}}, + } - routed := false - router.Handle("GET", "/user/:name", func(w http.ResponseWriter, r *http.Request, ps Params) { - routed = true - want := Params{Param{"name", "gopher"}} - if !reflect.DeepEqual(ps, want) { - t.Fatalf("wrong wildcard values: want %v, got %v", want, ps) - } - }) + for name, want := range testNames { + router := New() - w := new(mockResponseWriter) + routed := false + router.Handle("GET", "/user/:name", func(w http.ResponseWriter, r *http.Request, ps Params) { + routed = true + if !reflect.DeepEqual(ps, want) { + t.Fatalf("wrong wildcard values: want %v, got %v", want, ps) + } + }) - req, _ := http.NewRequest("GET", "/user/gopher", nil) - router.ServeHTTP(w, req) + w := new(mockResponseWriter) + + req, _ := http.NewRequest("GET", "/user/" + name, nil) + router.ServeHTTP(w, req) - if !routed { - t.Fatal("routing failed") + if !routed { + t.Fatal("routing failed") + } } } diff --git a/tree.go b/tree.go index b7355343..3e364400 100644 --- a/tree.go +++ b/tree.go @@ -364,7 +364,7 @@ walk: // outer loop for walking the tree i := len(p) p = p[:i+1] // expand slice within preallocated capacity p[i].Key = n.path[1:] - p[i].Value = path[:end] + p[i].Value = unencode(path[:end]) // we need to go deeper! if end < len(path) { @@ -647,3 +647,42 @@ walk: // outer loop for walking the tree } return ciPath, false } + +func unencode(s string) string { + unencoded := make([]byte, len(s)) + j := 0 + for i := 0; i < len(s); j++ { + if s[i] == '%' && i+2 < len(s) && ishex(s[i+1]) && ishex(s[i+2]) { + unencoded[j] = unhex(s[i+1])<<4 | unhex(s[i+2]) + i += 3 + } else { + unencoded[j] = s[i] + i++ + } + } + return string(unencoded[:j]) +} + +func ishex(c byte) bool { + switch { + case '0' <= c && c <= '9': + return true + case 'a' <= c && c <= 'f': + return true + case 'A' <= c && c <= 'F': + return true + } + return false +} + +func unhex(c byte) byte { + switch { + case '0' <= c && c <= '9': + return c - '0' + case 'a' <= c && c <= 'f': + return c - 'a' + 10 + case 'A' <= c && c <= 'F': + return c - 'A' + 10 + } + return 0 +} From 56ac744cf9d1929c6f0abd7848250305eeee0f46 Mon Sep 17 00:00:00 2001 From: Nicholas Rawlings Date: Fri, 13 May 2016 13:57:46 -0400 Subject: [PATCH 2/3] Restore compatibility with go1.4 and earlier --- requestpath_go15.go | 22 ++++++++++++++++++++++ requestpath_pre15.go | 27 +++++++++++++++++++++++++++ router.go | 10 +++++----- router_test.go | 4 ++-- 4 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 requestpath_go15.go create mode 100644 requestpath_pre15.go diff --git a/requestpath_go15.go b/requestpath_go15.go new file mode 100644 index 00000000..74f4dadc --- /dev/null +++ b/requestpath_go15.go @@ -0,0 +1,22 @@ +// +build go1.5 + +package httprouter + +import ( + "net/http" +) + +func getPath(r *http.Request) string { + return r.URL.EscapedPath() +} + +func addTrailingSlash(r *http.Request) { + r.URL.RawPath += "/" + r.URL.Path += "/" +} + +func removeTrailingSlash(r *http.Request) { + rawPath := r.URL.EscapedPath() + r.URL.RawPath = rawPath[:len(rawPath)-1] + r.URL.Path = r.URL.Path[:len(r.URL.Path)-1] +} diff --git a/requestpath_pre15.go b/requestpath_pre15.go new file mode 100644 index 00000000..e6ae2f8b --- /dev/null +++ b/requestpath_pre15.go @@ -0,0 +1,27 @@ +// +build !go1.5 + +package httprouter + +import ( + "net/http" + "strings" +) + +// BUG(): In go1.4 and earlier, percent-encoded slashes are only handled +// correctly for server requests. The http.NewRequest() function used to +// create client requests does not populate the RequestURI field. + +func getPath(r *http.Request) string { + if r.RequestURI != "" { + return strings.SplitN(r.RequestURI, "?", 2)[0] + } + return r.URL.Path +} + +func addTrailingSlash(r *http.Request) { + r.URL.Path = r.URL.Path + "/" +} + +func removeTrailingSlash(r *http.Request) { + r.URL.Path = r.URL.Path[:len(r.URL.Path)-1] +} diff --git a/router.go b/router.go index 99ffe9f6..055d1137 100644 --- a/router.go +++ b/router.go @@ -331,13 +331,15 @@ func (r *Router) allowed(path, reqMethod string) (allow string) { return } +// BUG(): Redirects do not preserve percent-encoded slashes in go1.4 and earlier. + // ServeHTTP makes the router implement the http.Handler interface. func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { if r.PanicHandler != nil { defer r.recv(w, req) } - path := req.URL.EscapedPath() + path := getPath(req) if root := r.trees[req.Method]; root != nil { if handle, ps, tsr := root.getValue(path); handle != nil { @@ -353,11 +355,9 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { if tsr && r.RedirectTrailingSlash { if len(path) > 1 && path[len(path)-1] == '/' { - req.URL.RawPath = path[:len(path)-1] - req.URL.Path = req.URL.Path[:len(req.URL.Path)-1] + removeTrailingSlash(req) } else { - req.URL.RawPath = path + "/" - req.URL.Path += "/" + addTrailingSlash(req) } http.Redirect(w, req, req.URL.String(), code) return diff --git a/router_test.go b/router_test.go index d017ad8a..4d2fb7bb 100644 --- a/router_test.go +++ b/router_test.go @@ -48,7 +48,7 @@ func TestParams(t *testing.T) { func TestRouter(t *testing.T) { testNames := map[string]Params{ "gopher": Params{Param{"name", "gopher"}}, - "go%2f1.6": Params{Param{"name", "go/1.6"}}, + "go%2f1.6": Params{Param{"name", "go/1.6"}}, // This case will fail in go1.4 and earlier } for name, want := range testNames { @@ -68,7 +68,7 @@ func TestRouter(t *testing.T) { router.ServeHTTP(w, req) if !routed { - t.Fatal("routing failed") + t.Fatal("routing failed on /user/" + name) } } } From c08d9de124fecc02ccef46fe70414d7a1a238dce Mon Sep 17 00:00:00 2001 From: Nicholas Rawlings Date: Fri, 13 May 2016 14:20:02 -0400 Subject: [PATCH 3/3] Manually populate RequestURI in tests --- router_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/router_test.go b/router_test.go index 4d2fb7bb..7318b751 100644 --- a/router_test.go +++ b/router_test.go @@ -48,7 +48,7 @@ func TestParams(t *testing.T) { func TestRouter(t *testing.T) { testNames := map[string]Params{ "gopher": Params{Param{"name", "gopher"}}, - "go%2f1.6": Params{Param{"name", "go/1.6"}}, // This case will fail in go1.4 and earlier + "go%2f1.6": Params{Param{"name", "go/1.6"}}, } for name, want := range testNames { @@ -65,6 +65,7 @@ func TestRouter(t *testing.T) { w := new(mockResponseWriter) req, _ := http.NewRequest("GET", "/user/" + name, nil) + req.RequestURI = "/user/" + name // Manually populate RequestURI to simulate a server request router.ServeHTTP(w, req) if !routed {