From edfe8fd088406750011a99cce693f367ba8633e9 Mon Sep 17 00:00:00 2001 From: Adam Eijdenberg Date: Wed, 18 Jan 2017 08:58:28 +1100 Subject: [PATCH] Fix issue 200, fail fast if regex is incorrectly specified using capturing groups. --- doc.go | 5 +++++ mux_test.go | 10 ++++++++++ regexp.go | 7 +++++++ 3 files changed, 22 insertions(+) diff --git a/doc.go b/doc.go index e9573dd8..00daf4a7 100644 --- a/doc.go +++ b/doc.go @@ -57,6 +57,11 @@ calling mux.Vars(): vars := mux.Vars(request) category := vars["category"] +Note that if any capturing groups are present, mux will panic() during parsing. To prevent +this, convert any capturing groups to non-capturing, e.g. change "/{sort:(asc|desc)}" to +"/{sort:(?:asc|desc)}". This is a change from prior versions which behaved unpredictably +when capturing groups were present. + And this is all you need to know about the basic usage. More advanced options are explained below. diff --git a/mux_test.go b/mux_test.go index b4b049ef..405aca6d 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1389,6 +1389,16 @@ func TestSubrouterErrorHandling(t *testing.T) { } } +// See: https://github.com/gorilla/mux/issues/200 +func TestPanicOnCapturingGroups(t *testing.T) { + defer func() { + if recover() == nil { + t.Errorf("(Test that capturing groups now fail fast) Expected panic, however test completed sucessfully.\n") + } + }() + NewRouter().NewRoute().Path("/{type:(promo|special)}/{promoId}.json") +} + // ---------------------------------------------------------------------------- // Helpers // ---------------------------------------------------------------------------- diff --git a/regexp.go b/regexp.go index fd8fe395..0189ad34 100644 --- a/regexp.go +++ b/regexp.go @@ -109,6 +109,13 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash, if errCompile != nil { return nil, errCompile } + + // Check for capturing groups which used to work in older versions + if reg.NumSubexp() != len(idxs)/2 { + panic(fmt.Sprintf("route %s contains capture groups in its regexp. ", template) + + "Only non-capturing groups are accepted: e.g. (?:pattern) instead of (pattern)") + } + // Done! return &routeRegexp{ template: template,