From 8ac5cf967fbadf2522625826070379e0fd6e2f98 Mon Sep 17 00:00:00 2001 From: Dave Newman Date: Tue, 20 May 2014 14:59:54 -0700 Subject: [PATCH 1/6] Add SkipClean option By default paths are run through the cleanPath method which prevents using fancier paths like /fetch/http://xkcd.com/534 This adds a SkipClean option so that this path isn't redirected to /fetch/http/xkcd.com/534 --- mux.go | 30 ++++++++++++++++++++++++------ route.go | 9 ++++++--- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/mux.go b/mux.go index fbb7f19..b399699 100644 --- a/mux.go +++ b/mux.go @@ -48,6 +48,8 @@ type Router struct { namedRoutes map[string]*Route // See Router.StrictSlash(). This defines the flag for new routes. strictSlash bool + // See Router.SkipClean(). This defines the flag for new routes. + skipClean bool // If true, do not clear the request context after handling the request KeepContext bool } @@ -73,8 +75,9 @@ func (r *Router) Match(req *http.Request, match *RouteMatch) bool { // When there is a match, the route variables can be retrieved calling // mux.Vars(request). func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { - // Clean path to canonical form and redirect. - if p := cleanPath(req.URL.Path); p != req.URL.Path { + if !r.skipClean { + // Clean path to canonical form and redirect. + if p := cleanPath(req.URL.Path); p != req.URL.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: @@ -83,9 +86,10 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { url.Path = p p = url.String() - w.Header().Set("Location", p) - w.WriteHeader(http.StatusMovedPermanently) - return + w.Header().Set("Location", p) + w.WriteHeader(http.StatusMovedPermanently) + return + } } var match RouteMatch var handler http.Handler @@ -133,6 +137,19 @@ func (r *Router) StrictSlash(value bool) *Router { return r } +// SkipClean defines the path cleaning behaviour for new routes. The initial +// value is false. +// +// When true, if the route path is "/path//to", it will remain with the double +// slash. This is helpful if you have a route like: /fetch/http://xkcd.com/534/ +// +// When false, the path will be cleaned, so /fetch/http://xkcd.com/534/ will +// become /fetch/http/xkcd.com/534 +func (r *Router) SkipClean(value bool) *Router { + r.skipClean = value + return r +} + // ---------------------------------------------------------------------------- // parentRoute // ---------------------------------------------------------------------------- @@ -170,7 +187,7 @@ func (r *Router) buildVars(m map[string]string) map[string]string { // NewRoute registers an empty route. func (r *Router) NewRoute() *Route { - route := &Route{parent: r, strictSlash: r.strictSlash} + route := &Route{parent: r, strictSlash: r.strictSlash, skipClean: r.skipClean} r.routes = append(r.routes, route) return route } @@ -357,6 +374,7 @@ func cleanPath(p string) string { if p[len(p)-1] == '/' && np != "/" { np += "/" } + return np } diff --git a/route.go b/route.go index bf92af2..6f1ef38 100644 --- a/route.go +++ b/route.go @@ -23,9 +23,12 @@ type Route struct { matchers []matcher // Manager for the variables from host and path. regexp *routeRegexpGroup - // If true, when the path pattern is "/path/", accessing "/path" will - // redirect to the former and vice versa. - strictSlash bool + // If true, when the path pattern is "/path/", accessing "/path" will + // redirect to the former and vice versa. + strictSlash bool + // If true, when the path pattern is "/path//to", accessing "/path//to" + // will not redirect + skipClean bool // If true, this route never matches: it is only used to build URLs. buildOnly bool // The name used to build URLs. From 786d36e5ab042d67efe94022439cf6c91ee711dc Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Mon, 7 Mar 2016 11:42:43 -0800 Subject: [PATCH 2/6] `go fmt` --- mux.go | 12 ++++++------ route.go | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/mux.go b/mux.go index b399699..c982c13 100644 --- a/mux.go +++ b/mux.go @@ -79,12 +79,12 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { // Clean path to canonical form and redirect. if p := cleanPath(req.URL.Path); p != req.URL.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: - // http://code.google.com/p/go/issues/detail?id=5252 - url := *req.URL - url.Path = p - p = url.String() + // 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() w.Header().Set("Location", p) w.WriteHeader(http.StatusMovedPermanently) diff --git a/route.go b/route.go index 6f1ef38..ca54610 100644 --- a/route.go +++ b/route.go @@ -23,12 +23,12 @@ type Route struct { matchers []matcher // Manager for the variables from host and path. regexp *routeRegexpGroup - // If true, when the path pattern is "/path/", accessing "/path" will - // redirect to the former and vice versa. - strictSlash bool - // If true, when the path pattern is "/path//to", accessing "/path//to" - // will not redirect - skipClean bool + // If true, when the path pattern is "/path/", accessing "/path" will + // redirect to the former and vice versa. + strictSlash bool + // If true, when the path pattern is "/path//to", accessing "/path//to" + // will not redirect + skipClean bool // If true, this route never matches: it is only used to build URLs. buildOnly bool // The name used to build URLs. From a41434aac388acdf2aa28ba41cde5cd998feedd3 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Mon, 2 May 2016 10:24:09 -0700 Subject: [PATCH 3/6] Clarify more of `SkipClean` --- mux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mux.go b/mux.go index c982c13..94f5ddd 100644 --- a/mux.go +++ b/mux.go @@ -138,7 +138,7 @@ func (r *Router) StrictSlash(value bool) *Router { } // SkipClean defines the path cleaning behaviour for new routes. The initial -// value is false. +// value is false. Users should be careful about which routes are not cleaned // // When true, if the route path is "/path//to", it will remain with the double // slash. This is helpful if you have a route like: /fetch/http://xkcd.com/534/ From 9935257381196ec7a0d4fd92fa0f61f24cd014bc Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Mon, 2 May 2016 10:24:18 -0700 Subject: [PATCH 4/6] Add test for `SkipClean` --- mux_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/mux_test.go b/mux_test.go index 8912d09..f147bb6 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1376,6 +1376,24 @@ func Test301Redirect(t *testing.T) { } } +func TestSkipClean(t *testing.T) { + func1 := func(w http.ResponseWriter, r *http.Request) {} + func2 := func(w http.ResponseWriter, r *http.Request) {} + + r := NewRouter() + r.SkipClean(true) + r.HandleFunc("/api/", func2).Name("func2") + r.HandleFunc("/", func1).Name("func1") + + req, _ := http.NewRequest("GET", "http://localhost//api/?abc=def", nil) + res := NewRecorder() + r.ServeHTTP(res, req) + + if len(res.HeaderMap["Location"]) != 0 { + t.Errorf("Shouldn't redirect since skip clean is disabled") + } +} + // https://plus.google.com/101022900381697718949/posts/eWy6DjFJ6uW func TestSubrouterHeader(t *testing.T) { expected := "func1 response" From 05d9d908e791c45f6deff3ea85fd6fd3a340a4c7 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Mon, 2 May 2016 10:24:47 -0700 Subject: [PATCH 5/6] Add `SkipClean()` to `*Route` --- route.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/route.go b/route.go index ca54610..6c53f9f 100644 --- a/route.go +++ b/route.go @@ -39,6 +39,10 @@ type Route struct { buildVarsFunc BuildVarsFunc } +func (r *Route) SkipClean() bool { + return r.skipClean +} + // Match matches the route against the request. func (r *Route) Match(req *http.Request, match *RouteMatch) bool { if r.buildOnly || r.err != nil { From 12a13f34e4fd6f7c8cbd499c446bded8d09ad8ad Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Mon, 2 May 2016 10:30:24 -0700 Subject: [PATCH 6/6] `go vet` is part of Go now --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4dcdacb..f4084bd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,9 +10,6 @@ matrix: - go: 1.6 - go: tip -install: - - go get golang.org/x/tools/cmd/vet - script: - go get -t -v ./... - diff -u <(echo -n) <(gofmt -d .)