From 884b5ffcbd3a11b730f0b75f5c86ac408753c34d Mon Sep 17 00:00:00 2001 From: Vivek V Date: Fri, 30 Aug 2019 17:41:56 +0530 Subject: [PATCH 01/10] Added capacity to slice creation, when capacity is known (#516) --- regexp.go | 2 +- route.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/regexp.go b/regexp.go index ac1abcd4..0293a928 100644 --- a/regexp.go +++ b/regexp.go @@ -195,7 +195,7 @@ func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool { // url builds a URL part using the given values. func (r *routeRegexp) url(values map[string]string) (string, error) { - urlValues := make([]interface{}, len(r.varsN)) + urlValues := make([]interface{}, len(r.varsN), len(r.varsN)) for k, v := range r.varsN { value, ok := values[v] if !ok { diff --git a/route.go b/route.go index 7343d78a..4098cb79 100644 --- a/route.go +++ b/route.go @@ -635,7 +635,7 @@ func (r *Route) GetQueriesRegexp() ([]string, error) { if r.regexp.queries == nil { return nil, errors.New("mux: route doesn't have queries") } - var queries []string + queries := make([]string, 0, len(r.regexp.queries)) for _, query := range r.regexp.queries { queries = append(queries, query.regexp.String()) } @@ -654,7 +654,7 @@ func (r *Route) GetQueriesTemplates() ([]string, error) { if r.regexp.queries == nil { return nil, errors.New("mux: route doesn't have queries") } - var queries []string + queries := make([]string, 0, len(r.regexp.queries)) for _, query := range r.regexp.queries { queries = append(queries, query.template) } From ff4e71f144166b1dfe3017a146f8ed32a82e688b Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 17 Oct 2019 17:48:19 -0700 Subject: [PATCH 02/10] Guess the scheme if r.URL.Scheme is unset (#474) * Guess the scheme if r.URL.Scheme is unset It's not expected that the request's URL is fully populated when used on the server-side (it's more of a client-side field), so we shouldn't expect it to be present. In practice, it's only rarely set at all on the server, making mux's `Schemes` matcher tricky to use as it is. This commit adds a test which would have failed before demonstrating the problem, as well as a fix which I think makes `.Schemes` match what users expect. * [doc] Add more detail to Schemes and URL godocs * Add route url test for schemes * Make httpserver test use more specific scheme matchers * Update test to have different responses per route --- mux_httpserver_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ mux_test.go | 6 ++---- old_test.go | 17 +++++---------- route.go | 32 ++++++++++++++++++++++++--- 4 files changed, 85 insertions(+), 19 deletions(-) create mode 100644 mux_httpserver_test.go diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go new file mode 100644 index 00000000..5d2f4d3a --- /dev/null +++ b/mux_httpserver_test.go @@ -0,0 +1,49 @@ +// +build go1.9 + +package mux + +import ( + "bytes" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" +) + +func TestSchemeMatchers(t *testing.T) { + router := NewRouter() + router.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) { + rw.Write([]byte("hello http world")) + }).Schemes("http") + router.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) { + rw.Write([]byte("hello https world")) + }).Schemes("https") + + assertResponseBody := func(t *testing.T, s *httptest.Server, expectedBody string) { + resp, err := s.Client().Get(s.URL) + if err != nil { + t.Fatalf("unexpected error getting from server: %v", err) + } + if resp.StatusCode != 200 { + t.Fatalf("expected a status code of 200, got %v", resp.StatusCode) + } + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("unexpected error reading body: %v", err) + } + if !bytes.Equal(body, []byte(expectedBody)) { + t.Fatalf("response should be hello world, was: %q", string(body)) + } + } + + t.Run("httpServer", func(t *testing.T) { + s := httptest.NewServer(router) + defer s.Close() + assertResponseBody(t, s, "hello http world") + }) + t.Run("httpsServer", func(t *testing.T) { + s := httptest.NewTLSServer(router) + defer s.Close() + assertResponseBody(t, s, "hello https world") + }) +} diff --git a/mux_test.go b/mux_test.go index edcee572..9a740bb8 100644 --- a/mux_test.go +++ b/mux_test.go @@ -11,6 +11,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/http/httptest" "net/url" "reflect" "strings" @@ -2895,10 +2896,7 @@ func newRequestWithHeaders(method, url string, headers ...string) *http.Request // newRequestHost a new request with a method, url, and host header func newRequestHost(method, url, host string) *http.Request { - req, err := http.NewRequest(method, url, nil) - if err != nil { - panic(err) - } + req := httptest.NewRequest(method, url, nil) req.Host = host return req } diff --git a/old_test.go b/old_test.go index b228983c..f088a951 100644 --- a/old_test.go +++ b/old_test.go @@ -385,6 +385,11 @@ var urlBuildingTests = []urlBuildingTest{ vars: []string{"subdomain", "foo", "category", "technology", "id", "42"}, url: "http://foo.domain.com/articles/technology/42", }, + { + route: new(Route).Host("example.com").Schemes("https", "http"), + vars: []string{}, + url: "https://example.com", + }, } func TestHeaderMatcher(t *testing.T) { @@ -502,18 +507,6 @@ func TestUrlBuilding(t *testing.T) { url := u.String() if url != v.url { t.Errorf("expected %v, got %v", v.url, url) - /* - reversePath := "" - reverseHost := "" - if v.route.pathTemplate != nil { - reversePath = v.route.pathTemplate.Reverse - } - if v.route.hostTemplate != nil { - reverseHost = v.route.hostTemplate.Reverse - } - - t.Errorf("%#v:\nexpected: %q\ngot: %q\nreverse path: %q\nreverse host: %q", v.route, v.url, url, reversePath, reverseHost) - */ } } diff --git a/route.go b/route.go index 4098cb79..750afe57 100644 --- a/route.go +++ b/route.go @@ -412,11 +412,30 @@ func (r *Route) Queries(pairs ...string) *Route { type schemeMatcher []string func (m schemeMatcher) Match(r *http.Request, match *RouteMatch) bool { - return matchInArray(m, r.URL.Scheme) + scheme := r.URL.Scheme + // https://golang.org/pkg/net/http/#Request + // "For [most] server requests, fields other than Path and RawQuery will be + // empty." + // Since we're an http muxer, the scheme is either going to be http or https + // though, so we can just set it based on the tls termination state. + if scheme == "" { + if r.TLS == nil { + scheme = "http" + } else { + scheme = "https" + } + } + return matchInArray(m, scheme) } // Schemes adds a matcher for URL schemes. // It accepts a sequence of schemes to be matched, e.g.: "http", "https". +// If the request's URL has a scheme set, it will be matched against. +// Generally, the URL scheme will only be set if a previous handler set it, +// such as the ProxyHeaders handler from gorilla/handlers. +// If unset, the scheme will be determined based on the request's TLS +// termination state. +// The first argument to Schemes will be used when constructing a route URL. func (r *Route) Schemes(schemes ...string) *Route { for k, v := range schemes { schemes[k] = strings.ToLower(v) @@ -493,8 +512,8 @@ func (r *Route) Subrouter() *Router { // This also works for host variables: // // r := mux.NewRouter() -// r.Host("{subdomain}.domain.com"). -// HandleFunc("/articles/{category}/{id:[0-9]+}", ArticleHandler). +// r.HandleFunc("/articles/{category}/{id:[0-9]+}", ArticleHandler). +// Host("{subdomain}.domain.com"). // Name("article") // // // url.String() will be "http://news.domain.com/articles/technology/42" @@ -502,6 +521,13 @@ func (r *Route) Subrouter() *Router { // "category", "technology", // "id", "42") // +// The scheme of the resulting url will be the first argument that was passed to Schemes: +// +// // url.String() will be "https://example.com" +// r := mux.NewRouter() +// url, err := r.Host("example.com") +// .Schemes("https", "http").URL() +// // All variables defined in the route are required, and their values must // conform to the corresponding patterns. func (r *Route) URL(pairs ...string) (*url.URL, error) { From f395758b854c4efa789b8c1e9b73479704c548cb Mon Sep 17 00:00:00 2001 From: Franklin Harding <32021905+fharding1@users.noreply.github.com> Date: Thu, 24 Oct 2019 05:12:56 -0700 Subject: [PATCH 03/10] Remove/cleanup request context helpers (#525) * Remove context helpers in context.go * Update request context funcs to take concrete types * Move TestNativeContextMiddleware to mux_test.go * Clarify KeepContext Go 1.7+ comment Mux doesn't build on Go < 1.7 so the comment doesn't really need to clarify anymore. --- context.go | 18 ------------------ context_test.go | 30 ------------------------------ mux.go | 22 ++++++++++++---------- mux_test.go | 24 ++++++++++++++++++++++++ test_helpers.go | 2 +- 5 files changed, 37 insertions(+), 59 deletions(-) delete mode 100644 context.go delete mode 100644 context_test.go diff --git a/context.go b/context.go deleted file mode 100644 index 665940a2..00000000 --- a/context.go +++ /dev/null @@ -1,18 +0,0 @@ -package mux - -import ( - "context" - "net/http" -) - -func contextGet(r *http.Request, key interface{}) interface{} { - return r.Context().Value(key) -} - -func contextSet(r *http.Request, key, val interface{}) *http.Request { - if val == nil { - return r - } - - return r.WithContext(context.WithValue(r.Context(), key, val)) -} diff --git a/context_test.go b/context_test.go deleted file mode 100644 index d8a56b42..00000000 --- a/context_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package mux - -import ( - "context" - "net/http" - "testing" - "time" -) - -func TestNativeContextMiddleware(t *testing.T) { - withTimeout := func(h http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx, cancel := context.WithTimeout(r.Context(), time.Minute) - defer cancel() - h.ServeHTTP(w, r.WithContext(ctx)) - }) - } - - r := NewRouter() - r.Handle("/path/{foo}", withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - vars := Vars(r) - if vars["foo"] != "bar" { - t.Fatal("Expected foo var to be set") - } - }))) - - rec := NewRecorder() - req := newRequest("GET", "/path/bar") - r.ServeHTTP(rec, req) -} diff --git a/mux.go b/mux.go index 26f9582a..c9ba6470 100644 --- a/mux.go +++ b/mux.go @@ -5,6 +5,7 @@ package mux import ( + "context" "errors" "fmt" "net/http" @@ -58,8 +59,7 @@ type Router struct { // If true, do not clear the request context after handling the request. // - // Deprecated: No effect when go1.7+ is used, since the context is stored - // on the request itself. + // Deprecated: No effect, since the context is stored on the request itself. KeepContext bool // Slice of middlewares to be called after a match is found @@ -195,8 +195,8 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { var handler http.Handler if r.Match(req, &match) { handler = match.Handler - req = setVars(req, match.Vars) - req = setCurrentRoute(req, match.Route) + req = requestWithVars(req, match.Vars) + req = requestWithRoute(req, match.Route) } if handler == nil && match.MatchErr == ErrMethodMismatch { @@ -426,7 +426,7 @@ const ( // Vars returns the route variables for the current request, if any. func Vars(r *http.Request) map[string]string { - if rv := contextGet(r, varsKey); rv != nil { + if rv := r.Context().Value(varsKey); rv != nil { return rv.(map[string]string) } return nil @@ -438,18 +438,20 @@ func Vars(r *http.Request) map[string]string { // after the handler returns, unless the KeepContext option is set on the // Router. func CurrentRoute(r *http.Request) *Route { - if rv := contextGet(r, routeKey); rv != nil { + if rv := r.Context().Value(routeKey); rv != nil { return rv.(*Route) } return nil } -func setVars(r *http.Request, val interface{}) *http.Request { - return contextSet(r, varsKey, val) +func requestWithVars(r *http.Request, vars map[string]string) *http.Request { + ctx := context.WithValue(r.Context(), varsKey, vars) + return r.WithContext(ctx) } -func setCurrentRoute(r *http.Request, val interface{}) *http.Request { - return contextSet(r, routeKey, val) +func requestWithRoute(r *http.Request, route *Route) *http.Request { + ctx := context.WithValue(r.Context(), routeKey, route) + return r.WithContext(ctx) } // ---------------------------------------------------------------------------- diff --git a/mux_test.go b/mux_test.go index 9a740bb8..1c906689 100644 --- a/mux_test.go +++ b/mux_test.go @@ -7,6 +7,7 @@ package mux import ( "bufio" "bytes" + "context" "errors" "fmt" "io/ioutil" @@ -16,6 +17,7 @@ import ( "reflect" "strings" "testing" + "time" ) func (r *Route) GoString() string { @@ -2804,6 +2806,28 @@ func TestSubrouterNotFound(t *testing.T) { } } +func TestContextMiddleware(t *testing.T) { + withTimeout := func(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), time.Minute) + defer cancel() + h.ServeHTTP(w, r.WithContext(ctx)) + }) + } + + r := NewRouter() + r.Handle("/path/{foo}", withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + vars := Vars(r) + if vars["foo"] != "bar" { + t.Fatal("Expected foo var to be set") + } + }))) + + rec := NewRecorder() + req := newRequest("GET", "/path/bar") + r.ServeHTTP(rec, req) +} + // mapToPairs converts a string map to a slice of string pairs func mapToPairs(m map[string]string) []string { var i int diff --git a/test_helpers.go b/test_helpers.go index 32ecffde..5f5c496d 100644 --- a/test_helpers.go +++ b/test_helpers.go @@ -15,5 +15,5 @@ import "net/http" // can be set by making a route that captures the required variables, // starting a server and sending the request to that server. func SetURLVars(r *http.Request, val map[string]string) *http.Request { - return setVars(r, val) + return requestWithVars(r, val) } From 946b6237eb8d0ce3225f502b7fd4208d0b60ce5f Mon Sep 17 00:00:00 2001 From: Franklin Harding Date: Thu, 14 Nov 2019 12:19:09 -0800 Subject: [PATCH 04/10] Fix the CORSMethodMiddleware bug with subrouters * Adds a test case for the repro given in issue #534 * Fixes the logic in CORSMethodMiddleware to handle matching routes better --- middleware.go | 25 ++++++++++--------------- middleware_test.go | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/middleware.go b/middleware.go index cf2b26dc..cb51c565 100644 --- a/middleware.go +++ b/middleware.go @@ -58,22 +58,17 @@ func CORSMethodMiddleware(r *Router) MiddlewareFunc { func getAllMethodsForRoute(r *Router, req *http.Request) ([]string, error) { var allMethods []string - err := r.Walk(func(route *Route, _ *Router, _ []*Route) error { - for _, m := range route.matchers { - if _, ok := m.(*routeRegexp); ok { - if m.Match(req, &RouteMatch{}) { - methods, err := route.GetMethods() - if err != nil { - return err - } - - allMethods = append(allMethods, methods...) - } - break + for _, route := range r.routes { + var match RouteMatch + if route.Match(req, &match) || match.MatchErr == ErrMethodMismatch { + methods, err := route.GetMethods() + if err != nil { + return nil, err } + + allMethods = append(allMethods, methods...) } - return nil - }) + } - return allMethods, err + return allMethods, nil } diff --git a/middleware_test.go b/middleware_test.go index 27647afe..e9f0ef55 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -478,6 +478,26 @@ func TestCORSMethodMiddleware(t *testing.T) { } } +func TestCORSMethodMiddlewareSubrouter(t *testing.T) { + router := NewRouter().StrictSlash(true) + + subrouter := router.PathPrefix("/test").Subrouter() + subrouter.HandleFunc("/hello", stringHandler("a")).Methods(http.MethodGet, http.MethodOptions, http.MethodPost) + subrouter.HandleFunc("/hello/{name}", stringHandler("b")).Methods(http.MethodGet, http.MethodOptions) + + subrouter.Use(CORSMethodMiddleware(subrouter)) + + rw := NewRecorder() + req := newRequest("GET", "/test/hello/asdf") + router.ServeHTTP(rw, req) + + actualMethods := rw.Header().Get("Access-Control-Allow-Methods") + expectedMethods := "GET,OPTIONS" + if actualMethods != expectedMethods { + t.Fatalf("expected methods %q but got: %q", expectedMethods, actualMethods) + } +} + func TestMiddlewareOnMultiSubrouter(t *testing.T) { first := "first" second := "second" From 4de8a5a4d283677c69afa1a86a044c8451633a18 Mon Sep 17 00:00:00 2001 From: wangming Date: Tue, 19 Nov 2019 21:02:14 +0800 Subject: [PATCH 05/10] fix headers regexp test (#536) --- mux_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mux_test.go b/mux_test.go index 1c906689..2d8d2b3e 100644 --- a/mux_test.go +++ b/mux_test.go @@ -684,8 +684,8 @@ func TestHeaders(t *testing.T) { }, { title: "Headers route, regex header values to match", - route: new(Route).Headers("foo", "ba[zr]"), - request: newRequestHeaders("GET", "http://localhost", map[string]string{"foo": "bar"}), + route: new(Route).HeadersRegexp("foo", "ba[zr]"), + request: newRequestHeaders("GET", "http://localhost", map[string]string{"foo": "baw"}), vars: map[string]string{}, host: "", path: "", From 49c01487a141b49f8ffe06277f3dca3ee80a55fa Mon Sep 17 00:00:00 2001 From: Veselkov Konstantin Date: Thu, 21 Nov 2019 21:05:00 +0400 Subject: [PATCH 06/10] lint: Remove golint warning (#526) --- regexp.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/regexp.go b/regexp.go index 0293a928..b5a15ed9 100644 --- a/regexp.go +++ b/regexp.go @@ -181,16 +181,16 @@ func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool { } } return r.regexp.MatchString(host) - } else { - if r.regexpType == regexpTypeQuery { - return r.matchQueryString(req) - } - path := req.URL.Path - if r.options.useEncodedPath { - path = req.URL.EscapedPath() - } - return r.regexp.MatchString(path) } + + if r.regexpType == regexpTypeQuery { + return r.matchQueryString(req) + } + path := req.URL.Path + if r.options.useEncodedPath { + path = req.URL.EscapedPath() + } + return r.regexp.MatchString(path) } // url builds a URL part using the given values. From 75dcda0896e109a2a22c9315bca3bb21b87b2ba5 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Sun, 12 Jan 2020 20:17:43 +0100 Subject: [PATCH 07/10] perf: reduce allocations in (*routeRegexp).getURLQuery (#544) A production server is seeing a significant amount of allocations in (*routeRegexp).getURLQuery Since it is only interested in a single value and only the first value we create a specialized function for that. Comparing a few parameter parsing scenarios: ``` Benchmark_findQueryKey/0-8 7184014 168 ns/op 0 B/op 0 allocs/op Benchmark_findQueryKey/1-8 5307873 227 ns/op 48 B/op 3 allocs/op Benchmark_findQueryKey/2-8 1560836 770 ns/op 483 B/op 10 allocs/op Benchmark_findQueryKey/3-8 1296200 931 ns/op 559 B/op 11 allocs/op Benchmark_findQueryKey/4-8 666502 1769 ns/op 3 B/op 1 allocs/op Benchmark_findQueryKeyGoLib/0-8 1740973 690 ns/op 864 B/op 8 allocs/op Benchmark_findQueryKeyGoLib/1-8 3029618 393 ns/op 432 B/op 4 allocs/op Benchmark_findQueryKeyGoLib/2-8 461427 2511 ns/op 1542 B/op 24 allocs/op Benchmark_findQueryKeyGoLib/3-8 324252 3804 ns/op 1984 B/op 28 allocs/op Benchmark_findQueryKeyGoLib/4-8 69348 14928 ns/op 12716 B/op 130 allocs/op ``` --- regexp.go | 45 ++++++++++++++++++++++--- regexp_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 regexp_test.go diff --git a/regexp.go b/regexp.go index b5a15ed9..96dd94ad 100644 --- a/regexp.go +++ b/regexp.go @@ -230,14 +230,51 @@ func (r *routeRegexp) getURLQuery(req *http.Request) string { return "" } templateKey := strings.SplitN(r.template, "=", 2)[0] - for key, vals := range req.URL.Query() { - if key == templateKey && len(vals) > 0 { - return key + "=" + vals[0] - } + val, ok := findFirstQueryKey(req.URL.RawQuery, templateKey) + if ok { + return templateKey + "=" + val } return "" } +// findFirstQueryKey returns the same result as (*url.URL).Query()[key][0]. +// If key was not found, empty string and false is returned. +func findFirstQueryKey(rawQuery, key string) (value string, ok bool) { + query := []byte(rawQuery) + for len(query) > 0 { + foundKey := query + if i := bytes.IndexAny(foundKey, "&;"); i >= 0 { + foundKey, query = foundKey[:i], foundKey[i+1:] + } else { + query = query[:0] + } + if len(foundKey) == 0 { + continue + } + var value []byte + if i := bytes.IndexByte(foundKey, '='); i >= 0 { + foundKey, value = foundKey[:i], foundKey[i+1:] + } + if len(foundKey) < len(key) { + // Cannot possibly be key. + continue + } + keyString, err := url.QueryUnescape(string(foundKey)) + if err != nil { + continue + } + if keyString != key { + continue + } + valueString, err := url.QueryUnescape(string(value)) + if err != nil { + continue + } + return valueString, true + } + return "", false +} + func (r *routeRegexp) matchQueryString(req *http.Request) bool { return r.regexp.MatchString(r.getURLQuery(req)) } diff --git a/regexp_test.go b/regexp_test.go new file mode 100644 index 00000000..0d80e6a5 --- /dev/null +++ b/regexp_test.go @@ -0,0 +1,91 @@ +package mux + +import ( + "net/url" + "reflect" + "strconv" + "testing" +) + +func Test_findFirstQueryKey(t *testing.T) { + tests := []string{ + "a=1&b=2", + "a=1&a=2&a=banana", + "ascii=%3Ckey%3A+0x90%3E", + "a=1;b=2", + "a=1&a=2;a=banana", + "a==", + "a=%2", + "a=20&%20%3F&=%23+%25%21%3C%3E%23%22%7B%7D%7C%5C%5E%5B%5D%60%E2%98%BA%09:%2F@$%27%28%29%2A%2C%3B&a=30", + "a=1& ?&=#+%!<>#\"{}|\\^[]`☺\t:/@$'()*,;&a=5", + "a=xxxxxxxxxxxxxxxx&b=YYYYYYYYYYYYYYY&c=ppppppppppppppppppp&f=ttttttttttttttttt&a=uuuuuuuuuuuuu", + } + for _, query := range tests { + t.Run(query, func(t *testing.T) { + // Check against url.ParseQuery, ignoring the error. + all, _ := url.ParseQuery(query) + for key, want := range all { + t.Run(key, func(t *testing.T) { + got, ok := findFirstQueryKey(query, key) + if !ok { + t.Error("Did not get expected key", key) + } + if !reflect.DeepEqual(got, want[0]) { + t.Errorf("findFirstQueryKey(%s,%s) = %v, want %v", query, key, got, want[0]) + } + }) + } + }) + } +} + +func Benchmark_findQueryKey(b *testing.B) { + tests := []string{ + "a=1&b=2", + "ascii=%3Ckey%3A+0x90%3E", + "a=20&%20%3F&=%23+%25%21%3C%3E%23%22%7B%7D%7C%5C%5E%5B%5D%60%E2%98%BA%09:%2F@$%27%28%29%2A%2C%3B&a=30", + "a=xxxxxxxxxxxxxxxx&bbb=YYYYYYYYYYYYYYY&cccc=ppppppppppppppppppp&ddddd=ttttttttttttttttt&a=uuuuuuuuuuuuu", + "a=;b=;c=;d=;e=;f=;g=;h=;i=,j=;k=", + } + for i, query := range tests { + b.Run(strconv.Itoa(i), func(b *testing.B) { + // Check against url.ParseQuery, ignoring the error. + all, _ := url.ParseQuery(query) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + for key, _ := range all { + _, _ = findFirstQueryKey(query, key) + } + } + }) + } +} + +func Benchmark_findQueryKeyGoLib(b *testing.B) { + tests := []string{ + "a=1&b=2", + "ascii=%3Ckey%3A+0x90%3E", + "a=20&%20%3F&=%23+%25%21%3C%3E%23%22%7B%7D%7C%5C%5E%5B%5D%60%E2%98%BA%09:%2F@$%27%28%29%2A%2C%3B&a=30", + "a=xxxxxxxxxxxxxxxx&bbb=YYYYYYYYYYYYYYY&cccc=ppppppppppppppppppp&ddddd=ttttttttttttttttt&a=uuuuuuuuuuuuu", + "a=;b=;c=;d=;e=;f=;g=;h=;i=,j=;k=", + } + for i, query := range tests { + b.Run(strconv.Itoa(i), func(b *testing.B) { + // Check against url.ParseQuery, ignoring the error. + all, _ := url.ParseQuery(query) + var u url.URL + u.RawQuery = query + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + for key, _ := range all { + v := u.Query()[key] + if len(v) > 0 { + _ = v[0] + } + } + } + }) + } +} From 948bec34b5168796bf3fc4dbb09215baa970351a Mon Sep 17 00:00:00 2001 From: Eric Skoglund Date: Sun, 17 May 2020 06:02:54 +0200 Subject: [PATCH 08/10] docs: Remove stale text from comment. (#568) Comment for CurrentRoute claimed that setting the KeepContext option would propagate the Route even after the request. The KeepContext option is deprecated and has no effect. --- mux.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mux.go b/mux.go index c9ba6470..782a34b2 100644 --- a/mux.go +++ b/mux.go @@ -435,8 +435,7 @@ func Vars(r *http.Request) map[string]string { // CurrentRoute returns the matched route for the current request, if any. // This only works when called inside the handler of the matched route // because the matched route is stored in the request context which is cleared -// after the handler returns, unless the KeepContext option is set on the -// Router. +// after the handler returns. func CurrentRoute(r *http.Request) *Route { if rv := r.Context().Value(routeKey); rv != nil { return rv.(*Route) From 98cb6bf42e086f6af920b965c38cacc07402d51b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 11 Jul 2020 13:05:21 -0700 Subject: [PATCH 09/10] fix: regression in vars extract for wildcard host (#579) Continuing from PR #447 we have to add extra check to ignore the port as well add tests to cover this case --- old_test.go | 23 ++++++++++++++++++++++- regexp.go | 6 ++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/old_test.go b/old_test.go index f088a951..96dbe337 100644 --- a/old_test.go +++ b/old_test.go @@ -260,6 +260,18 @@ var hostMatcherTests = []hostMatcherTest{ vars: map[string]string{"foo": "abc", "bar": "def", "baz": "ghi"}, result: true, }, + { + matcher: NewRouter().NewRoute().Host("{foo:[a-z][a-z][a-z]}.{bar:[a-z][a-z][a-z]}.{baz:[a-z][a-z][a-z]}:{port:.*}"), + url: "http://abc.def.ghi:65535/", + vars: map[string]string{"foo": "abc", "bar": "def", "baz": "ghi", "port": "65535"}, + result: true, + }, + { + matcher: NewRouter().NewRoute().Host("{foo:[a-z][a-z][a-z]}.{bar:[a-z][a-z][a-z]}.{baz:[a-z][a-z][a-z]}"), + url: "http://abc.def.ghi:65535/", + vars: map[string]string{"foo": "abc", "bar": "def", "baz": "ghi"}, + result: true, + }, { matcher: NewRouter().NewRoute().Host("{foo:[a-z][a-z][a-z]}.{bar:[a-z][a-z][a-z]}.{baz:[a-z][a-z][a-z]}"), url: "http://a.b.c/", @@ -365,6 +377,11 @@ var urlBuildingTests = []urlBuildingTest{ vars: []string{"subdomain", "bar"}, url: "http://bar.domain.com", }, + { + route: new(Route).Host("{subdomain}.domain.com:{port:.*}"), + vars: []string{"subdomain", "bar", "port", "65535"}, + url: "http://bar.domain.com:65535", + }, { route: new(Route).Host("foo.domain.com").Path("/articles"), vars: []string{}, @@ -412,7 +429,11 @@ func TestHeaderMatcher(t *testing.T) { func TestHostMatcher(t *testing.T) { for _, v := range hostMatcherTests { - request, _ := http.NewRequest("GET", v.url, nil) + request, err := http.NewRequest("GET", v.url, nil) + if err != nil { + t.Errorf("http.NewRequest failed %#v", err) + continue + } var routeMatch RouteMatch result := v.matcher.Match(request, &routeMatch) vars := routeMatch.Vars diff --git a/regexp.go b/regexp.go index 96dd94ad..0144842b 100644 --- a/regexp.go +++ b/regexp.go @@ -325,6 +325,12 @@ func (v routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route) { // Store host variables. if v.host != nil { host := getHost(req) + if v.host.wildcardHostPort { + // Don't be strict on the port match + if i := strings.Index(host, ":"); i != -1 { + host = host[:i] + } + } matches := v.host.regexp.FindStringSubmatchIndex(host) if len(matches) > 0 { extractVars(host, matches, v.host.varsN, m.Vars) From d07530f46e1eec4e40346e24af34dcc6750ad39f Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Sat, 12 Sep 2020 12:20:56 -0700 Subject: [PATCH 10/10] build: CircleCI 2.1 + build matrix (#595) * build: CircleCI 2.1 + build matrix * build: drop Go 1.9 & Go 1.10 * build: remove erroneous version --- .circleci/config.yml | 103 ++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 60 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 536bc119..ead3e1d4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,87 +1,70 @@ -version: 2.0 +version: 2.1 jobs: - # Base test configuration for Go library tests Each distinct version should - # inherit this base, and override (at least) the container image used. - "test": &test + "test": + parameters: + version: + type: string + default: "latest" + golint: + type: boolean + default: true + modules: + type: boolean + default: true + goproxy: + type: string + default: "" docker: - - image: circleci/golang:latest + - image: "circleci/golang:<< parameters.version >>" working_directory: /go/src/github.com/gorilla/mux - steps: &steps - # Our build steps: we checkout the repo, fetch our deps, lint, and finally - # run "go test" on the package. + environment: + GO111MODULE: "on" + GOPROXY: "<< parameters.goproxy >>" + steps: - checkout - # Logs the version in our build logs, for posterity - - run: go version + - run: + name: "Print the Go version" + command: > + go version - run: name: "Fetch dependencies" command: > - go get -t -v ./... + if [[ << parameters.modules >> = true ]]; then + go mod download + export GO111MODULE=on + else + go get -v ./... + fi # Only run gofmt, vet & lint against the latest Go version - run: name: "Run golint" command: > - if [ "${LATEST}" = true ] && [ -z "${SKIP_GOLINT}" ]; then + if [ << parameters.version >> = "latest" ] && [ << parameters.golint >> = true ]; then go get -u golang.org/x/lint/golint golint ./... fi - run: name: "Run gofmt" command: > - if [[ "${LATEST}" = true ]]; then + if [[ << parameters.version >> = "latest" ]]; then diff -u <(echo -n) <(gofmt -d -e .) fi - run: name: "Run go vet" - command: > - if [[ "${LATEST}" = true ]]; then + command: > + if [[ << parameters.version >> = "latest" ]]; then go vet -v ./... fi - - run: go test -v -race ./... - - "latest": - <<: *test - environment: - LATEST: true - - "1.12": - <<: *test - docker: - - image: circleci/golang:1.12 - - "1.11": - <<: *test - docker: - - image: circleci/golang:1.11 - - "1.10": - <<: *test - docker: - - image: circleci/golang:1.10 - - "1.9": - <<: *test - docker: - - image: circleci/golang:1.9 - - "1.8": - <<: *test - docker: - - image: circleci/golang:1.8 - - "1.7": - <<: *test - docker: - - image: circleci/golang:1.7 + - run: + name: "Run go test (+ race detector)" + command: > + go test -v -race ./... workflows: - version: 2 - build: + tests: jobs: - - "latest" - - "1.12" - - "1.11" - - "1.10" - - "1.9" - - "1.8" - - "1.7" + - test: + matrix: + parameters: + version: ["latest", "1.15", "1.14", "1.13", "1.12", "1.11"]