Add useEncodedPath option to router and routes (#190)

- Resolves a breaking change in #184

Author: kushmansingh

mux.go

@@ -53,6 +53,8 @@ type Router struct {
 	// This has no effect when go1.7+ is used, since the context is stored
 	// on the request itself.
 	KeepContext bool
+	// see Router.UseEncodedPath(). This defines a flag for all routes.
+	useEncodedPath bool
 }
 
 // Match matches registered routes against the request.
@@ -77,7 +79,10 @@ 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)
+		path := req.URL.Path
+		if r.useEncodedPath {
+			path = getPath(req)
+		}
 		// Clean path to canonical form and redirect.
 		if p := cleanPath(path); p != path {
 
@@ -152,6 +157,21 @@ func (r *Router) SkipClean(value bool) *Router {
 	return r
 }
 
+// UseEncodedPath tells the router to match the encoded original path
+// to the routes.
+// For eg. "/path/foo%2Fbar/to" will match the path "/path/{var}/to".
+// This behavior has the drawback of needing to match routes against
+// r.RequestURI instead of r.URL.Path. Any modifications (such as http.StripPrefix)
+// to r.URL.Path will not affect routing when this flag is on and thus may
+// induce unintended behavior.
+//
+// If not called, the router will match the unencoded path to the routes.
+// For eg. "/path/foo%2Fbar/to" will match the path "/path/foo/bar/to"
+func (r *Router) UseEncodedPath() *Router {
+	r.useEncodedPath = true
+	return r
+}
+
 // ----------------------------------------------------------------------------
 // parentRoute
 // ----------------------------------------------------------------------------
@@ -189,7 +209,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, skipClean: r.skipClean}
+	route := &Route{parent: r, strictSlash: r.strictSlash, skipClean: r.skipClean, useEncodedPath: r.useEncodedPath}
 	r.routes = append(r.routes, route)
 	return route
 }

mux_test.go

@@ -335,16 +335,6 @@ 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}}"),
@@ -430,6 +420,7 @@ func TestPath(t *testing.T) {
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
 	}
 }
 
@@ -507,6 +498,7 @@ func TestPathPrefix(t *testing.T) {
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
 	}
 }
 
@@ -583,6 +575,7 @@ func TestHostPath(t *testing.T) {
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
 	}
 }
 
@@ -909,6 +902,7 @@ func TestQueries(t *testing.T) {
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
 	}
 }
 
@@ -1068,6 +1062,7 @@ func TestSubRouter(t *testing.T) {
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
 	}
 }
 
@@ -1161,6 +1156,40 @@ func TestStrictSlash(t *testing.T) {
 		},
 	}
 
+	for _, test := range tests {
+		testRoute(t, test)
+		testTemplate(t, test)
+		testUseEscapedRoute(t, test)
+	}
+}
+
+func TestUseEncodedPath(t *testing.T) {
+	r := NewRouter()
+	r.UseEncodedPath()
+
+	tests := []routeTest{
+		{
+			title:        "Router with useEncodedPath, URL with encoded slash does match",
+			route:        r.NewRoute().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:        "Router with useEncodedPath, URL with encoded slash doesn't match",
+			route:        r.NewRoute().Path("/v1/1/2/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/1/2/v2`,
+			shouldMatch:  false,
+		},
+	}
+
 	for _, test := range tests {
 		testRoute(t, test)
 		testTemplate(t, test)
@@ -1375,6 +1404,11 @@ func testRoute(t *testing.T, test routeTest) {
 	}
 }
 
+func testUseEscapedRoute(t *testing.T, test routeTest) {
+	test.route.useEncodedPath = true
+	testRoute(t, test)
+}
+
 func testTemplate(t *testing.T, test routeTest) {
 	route := test.route
 	pathTemplate := test.pathTemplate

old_test.go

@@ -687,7 +687,7 @@ func TestNewRegexp(t *testing.T) {
 	}
 
 	for pattern, paths := range tests {
-		p, _ = newRouteRegexp(pattern, false, false, false, false)
+		p, _ = newRouteRegexp(pattern, false, false, false, false, false)
 		for path, result := range paths {
 			matches = p.regexp.FindStringSubmatch(path)
 			if result == nil {

regexp.go

@@ -24,7 +24,7 @@ import (
 // Previously we accepted only Python-like identifiers for variable
 // names ([a-zA-Z_][a-zA-Z0-9_]*), but currently the only restriction is that
 // name and pattern can't be empty, and names can't contain a colon.
-func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash bool) (*routeRegexp, error) {
+func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash, useEncodedPath bool) (*routeRegexp, error) {
 	// Check if it is well-formed.
 	idxs, errBraces := braceIndices(tpl)
 	if errBraces != nil {
@@ -111,14 +111,15 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash
 	}
 	// Done!
 	return &routeRegexp{
-		template:    template,
-		matchHost:   matchHost,
-		matchQuery:  matchQuery,
-		strictSlash: strictSlash,
-		regexp:      reg,
-		reverse:     reverse.String(),
-		varsN:       varsN,
-		varsR:       varsR,
+		template:       template,
+		matchHost:      matchHost,
+		matchQuery:     matchQuery,
+		strictSlash:    strictSlash,
+		useEncodedPath: useEncodedPath,
+		regexp:         reg,
+		reverse:        reverse.String(),
+		varsN:          varsN,
+		varsR:          varsR,
 	}, nil
 }
 
@@ -133,6 +134,9 @@ type routeRegexp struct {
 	matchQuery bool
 	// The strictSlash value defined on the route, but disabled if PathPrefix was used.
 	strictSlash bool
+	// Determines whether to use encoded path from getPath function or unencoded
+	// req.URL.Path for path matching
+	useEncodedPath bool
 	// Expanded regexp.
 	regexp *regexp.Regexp
 	// Reverse template.
@@ -149,7 +153,10 @@ func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool {
 		if r.matchQuery {
 			return r.matchQueryString(req)
 		}
-		path := getPath(req)
+		path := req.URL.Path
+		if r.useEncodedPath {
+			path = getPath(req)
+		}
 		return r.regexp.MatchString(path)
 	}
 
@@ -253,7 +260,10 @@ func (v *routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route)
 			extractVars(host, matches, v.host.varsN, m.Vars)
 		}
 	}
-	path := getPath(req)
+	path := req.URL.Path
+	if r.useEncodedPath {
+		path = getPath(req)
+	}
 	// Store path variables.
 	if v.path != nil {
 		matches := v.path.regexp.FindStringSubmatchIndex(path)

route.go

@@ -29,6 +29,8 @@ type Route struct {
 	// If true, when the path pattern is "/path//to", accessing "/path//to"
 	// will not redirect
 	skipClean bool
+	// If true, "/path/foo%2Fbar/to" will match the path "/path/{var}/to"
+	useEncodedPath bool
 	// If true, this route never matches: it is only used to build URLs.
 	buildOnly bool
 	// The name used to build URLs.
@@ -158,7 +160,7 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
 			tpl = strings.TrimRight(r.regexp.path.template, "/") + tpl
 		}
 	}
-	rr, err := newRouteRegexp(tpl, matchHost, matchPrefix, matchQuery, r.strictSlash)
+	rr, err := newRouteRegexp(tpl, matchHost, matchPrefix, matchQuery, r.strictSlash, r.useEncodedPath)
 	if err != nil {
 		return err
 	}