Skip to content

Commit

Permalink
[bugfix] fail fast if regex is incorrectly specified using capturing …
Browse files Browse the repository at this point in the history
…groups. (#218)
  • Loading branch information
aeijdenberg authored and elithrar committed Jan 18, 2017
1 parent cafdb65 commit 392c28f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
5 changes: 5 additions & 0 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ----------------------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 392c28f

Please sign in to comment.