Repository: gorilla/mux (go)

Issue:
Host() with  a port fails to match when the host is supplied in the HTTP Host: header
Old issue: http://code.google.com/p/gorilla/issues/detail?id=51

According to section 14.23 of RFC 2616 the Host header can include the port number if the default value of 80 is not used.

See https://github.com/gorilla/mux/blob/master/mux_test.go#L119 for a commented-out test case that fails.

Potential fix is editing getHost() https://github.com/gorilla/mux/blob/master/regexp.go#L237 so it has a flag as to whether to strip off the port information. A new field would need to be probably need to be added somewhere such as routeRegexp.

The main reason I was using Host() match with a port in the first place is for trying to build reverse routes that include the absolute path as opposed to a relative one.


Pull request:
fix #2, return full host:port info from getHost
I ran into #2, i think its safe to simply `return r.Host` because `r.URL.IsAbs()` case is already doing that and is covered by existing tests cases.

Patch:
diff --git a/README.md b/README.md
index 0425bb8..9e99a27 100644
--- a/README.md
+++ b/README.md
@@ -88,7 +88,7 @@ r := mux.NewRouter()
 // Only matches if domain is "www.example.com".
 r.Host("www.example.com")
 // Matches a dynamic subdomain.
-r.Host("{subdomain:[a-z]+}.domain.com")
+r.Host("{subdomain:[a-z]+}.example.com")
 ```
 
 There are several other matchers that can be added. To match path prefixes:
@@ -238,13 +238,13 @@ This also works for host and query value variables:
 
 ```go
 r := mux.NewRouter()
-r.Host("{subdomain}.domain.com").
+r.Host("{subdomain}.example.com").
   Path("/articles/{category}/{id:[0-9]+}").
   Queries("filter", "{filter}").
   HandlerFunc(ArticleHandler).
   Name("article")
 
-// url.String() will be "http://news.domain.com/articles/technology/42?filter=gorilla"
+// url.String() will be "http://news.example.com/articles/technology/42?filter=gorilla"
 url, err := r.Get("article").URL("subdomain", "news",
                                  "category", "technology",
                                  "id", "42",
@@ -264,7 +264,7 @@ r.HeadersRegexp("Content-Type", "application/(text|json)")
 There's also a way to build only the URL host or path for a route: use the methods `URLHost()` or `URLPath()` instead. For the previous route, we would do:
 
 ```go
-// "http://news.domain.com/"
+// "http://news.example.com/"
 host, err := r.Get("article").URLHost("subdomain", "news")
 
 // "/articles/technology/42"
@@ -275,12 +275,12 @@ And if you use subrouters, host and path defined separately can be built as well
 
 ```go
 r := mux.NewRouter()
-s := r.Host("{subdomain}.domain.com").Subrouter()
+s := r.Host("{subdomain}.example.com").Subrouter()
 s.Path("/articles/{category}/{id:[0-9]+}").
   HandlerFunc(ArticleHandler).
   Name("article")
 
-// "http://news.domain.com/articles/technology/42"
+// "http://news.example.com/articles/technology/42"
 url, err := r.Get("article").URL("subdomain", "news",
                                  "category", "technology",
                                  "id", "42")
diff --git a/context.go b/context.go
index 13a4601..665940a 100644
--- a/context.go
+++ b/context.go
@@ -16,7 +16,3 @@ func contextSet(r *http.Request, key, val interface{}) *http.Request {
 
 	return r.WithContext(context.WithValue(r.Context(), key, val))
 }
-
-func contextClear(r *http.Request) {
-	return
-}
diff --git a/mux.go b/mux.go
index 50ac118..8aca972 100644
--- a/mux.go
+++ b/mux.go
@@ -22,7 +22,7 @@ var (
 
 // NewRouter returns a new router instance.
 func NewRouter() *Router {
-	return &Router{namedRoutes: make(map[string]*Route), KeepContext: false}
+	return &Router{namedRoutes: make(map[string]*Route)}
 }
 
 // Router registers routes to be matched and dispatches a handler.
@@ -57,7 +57,8 @@ type Router struct {
 	namedRoutes map[string]*Route
 
 	// If true, do not clear the request context after handling the request.
-	// This has no effect when go1.7+ is used, since the context is stored
+	//
+	// Deprecated: No effect when go1.7+ is used, since the context is stored
 	// on the request itself.
 	KeepContext bool
 
@@ -208,10 +209,6 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		handler = http.NotFoundHandler()
 	}
 
-	if !r.KeepContext {
-		defer contextClear(req)
-	}
-
 	handler.ServeHTTP(w, req)
 }
 
diff --git a/regexp.go b/regexp.go
index 7c7405d..f252886 100644
--- a/regexp.go
+++ b/regexp.go
@@ -312,17 +312,13 @@ func (v routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route) {
 }
 
 // getHost tries its best to return the request host.
+// According to section 14.23 of RFC 2616 the Host header
+// can include the port number if the default value of 80 is not used.
 func getHost(r *http.Request) string {
 	if r.URL.IsAbs() {
 		return r.URL.Host
 	}
-	host := r.Host
-	// Slice off any port information.
-	if i := strings.Index(host, ":"); i != -1 {
-		host = host[:i]
-	}
-	return host
-
+	return r.Host
 }
 
 func extractVars(input string, matches []int, names []string, output map[string]string) {
diff --git a/route.go b/route.go
index acef919..a197096 100644
--- a/route.go
+++ b/route.go
@@ -43,6 +43,11 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {
 		return false
 	}
 
+	// Set MatchErr to nil to prevent
+	// subsequent matching subrouters from failing to run middleware.
+	// If not reset, the middleware would see a non-nil MatchErr and be skipped,
+	// even when there was a matching route.
+	match.MatchErr = nil
 	var matchErr error
 
 	// Match everything.


Test patch:
diff --git a/middleware_test.go b/middleware_test.go
index acf4e16..b708be0 100644
--- a/middleware_test.go
+++ b/middleware_test.go
@@ -375,3 +375,63 @@ func TestCORSMethodMiddleware(t *testing.T) {
 		}
 	}
 }
+
+func TestMiddlewareOnMultiSubrouter(t *testing.T) {
+	first := "first"
+	second := "second"
+	notFound := "404 not found"
+
+	router := NewRouter()
+	firstSubRouter := router.PathPrefix("/").Subrouter()
+	secondSubRouter := router.PathPrefix("/").Subrouter()
+
+	router.NotFoundHandler = http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.Write([]byte(notFound))
+	})
+
+	firstSubRouter.HandleFunc("/first", func(w http.ResponseWriter, r *http.Request) {
+
+	})
+
+	secondSubRouter.HandleFunc("/second", func(w http.ResponseWriter, r *http.Request) {
+
+	})
+
+	firstSubRouter.Use(func(h http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte(first))
+			h.ServeHTTP(w, r)
+		})
+	})
+
+	secondSubRouter.Use(func(h http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte(second))
+			h.ServeHTTP(w, r)
+		})
+	})
+
+	rw := NewRecorder()
+	req := newRequest("GET", "/first")
+
+	router.ServeHTTP(rw, req)
+	if rw.Body.String() != first {
+		t.Fatalf("Middleware did not run: expected %s middleware to write a response (got %s)", first, rw.Body.String())
+	}
+
+	rw = NewRecorder()
+	req = newRequest("GET", "/second")
+
+	router.ServeHTTP(rw, req)
+	if rw.Body.String() != second {
+		t.Fatalf("Middleware did not run: expected %s middleware to write a response (got %s)", second, rw.Body.String())
+	}
+
+	rw = NewRecorder()
+	req = newRequest("GET", "/second/not-exist")
+
+	router.ServeHTTP(rw, req)
+	if rw.Body.String() != notFound {
+		t.Fatalf("Notfound handler did not run: expected %s for not-exist, (got %s)", notFound, rw.Body.String())
+	}
+}
diff --git a/mux_test.go b/mux_test.go
index 519aa92..8ad57ac 100644
--- a/mux_test.go
+++ b/mux_test.go
@@ -104,7 +104,15 @@ func TestHost(t *testing.T) {
 			path:        "",
 			shouldMatch: false,
 		},
-		// BUG {new(Route).Host("aaa.bbb.ccc:1234"), newRequestHost("GET", "/111/222/333", "aaa.bbb.ccc:1234"), map[string]string{}, "aaa.bbb.ccc:1234", "", true},
+		{
+			title:       "Host route with port, match with request header",
+			route:       new(Route).Host("aaa.bbb.ccc:1234"),
+			request:     newRequestHost("GET", "/111/222/333", "aaa.bbb.ccc:1234"),
+			vars:        map[string]string{},
+			host:        "aaa.bbb.ccc:1234",
+			path:        "",
+			shouldMatch: true,
+		},
 		{
 			title:       "Host route with port, wrong host in request header",
 			route:       new(Route).Host("aaa.bbb.ccc:1234"),
@@ -114,6 +122,16 @@ func TestHost(t *testing.T) {
 			path:        "",
 			shouldMatch: false,
 		},
+		{
+			title:        "Host route with pattern, match with request header",
+			route:        new(Route).Host("aaa.{v1:[a-z]{3}}.ccc:1{v2:(?:23|4)}"),
+			request:      newRequestHost("GET", "/111/222/333", "aaa.bbb.ccc:123"),
+			vars:         map[string]string{"v1": "bbb", "v2": "23"},
+			host:         "aaa.bbb.ccc:123",
+			path:         "",
+			hostTemplate: `aaa.{v1:[a-z]{3}}.ccc:1{v2:(?:23|4)}`,
+			shouldMatch:  true,
+		},
 		{
 			title:        "Host route with pattern, match",
 			route:        new(Route).Host("aaa.{v1:[a-z]{3}}.ccc"),