From ebbb49df897b87d0a568fe21f00b85a1d4af7a50 Mon Sep 17 00:00:00 2001 From: Charles Iliya Krempeaux Date: Mon, 24 Jun 2019 14:17:57 -0700 Subject: [PATCH] there was a bug in pathmatch.Find(). it allowed extra stuff at ne, and basically only matched beginning of pattern. correct it. --- pattern_find.go | 36 +++++--- pattern_match_test.go | 209 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 13 deletions(-) diff --git a/pattern_find.go b/pattern_find.go index 1ef8e56..a0e16a3 100644 --- a/pattern_find.go +++ b/pattern_find.go @@ -12,6 +12,11 @@ var ( errThisShouldNeverHappen = newInternalError("This should never happen.") ) +// Find compares ‘path’ against its (compiled) pattern template; if it matches it loads the +// matches into ‘args’, and then returns true. +// +// Find may set some, or all of the items in ‘args’ even if it returns false, and even if it +// returns an error. func (pattern *Pattern) Find(path string, args ...interface{}) (bool, error) { if nil == pattern { return false, errNilReceiver @@ -34,23 +39,28 @@ func (pattern *Pattern) Find(path string, args ...interface{}) (bool, error) { s = s[len(bit):] case wildcardBit: index := strings.IndexRune(s, '/') - if -1 == index { - if err := set(s, argsIndex, args...); nil != err { - return doesNotMatter, err - } - argsIndex++ - } else if 0 <= index { - value := s[:index] - if err := set(value, argsIndex, args...); nil != err { - return doesNotMatter, err - } - argsIndex++ - s = s[index:] - } else { + + var value string + switch { + default: return doesNotMatter, errThisShouldNeverHappen + case -1 == index: + value = s + case 0 <= index: + value = s[:index] } + + if err := set(value, argsIndex, args...); nil != err { + return doesNotMatter, err + } + argsIndex++ + s = s[len(value):] } } + if "" != s { + return false, nil + } + return true, nil } diff --git a/pattern_match_test.go b/pattern_match_test.go index b202407..ecacd60 100644 --- a/pattern_match_test.go +++ b/pattern_match_test.go @@ -17,6 +17,11 @@ func TestPatternMatch(t *testing.T) { Path: "/v1/help", Expected: true, }, + { + Pattern: "/v1/help", + Path: "/v1/help/", + Expected: false, + }, { Pattern: "/v1/help", Path: "/v2/help", @@ -45,11 +50,150 @@ func TestPatternMatch(t *testing.T) { + { + Pattern: "/v1/help/", + Path: "/v1/help/", + Expected: true, + }, + { + Pattern: "/v1/help/", + Path: "/v1/help", + Expected: false, + }, + { + Pattern: "/v1/help/", + Path: "/v2/help/", + Expected: false, + }, + { + Pattern: "/v1/help/", + Path: "/v2/HELP/", + Expected: false, + }, + { + Pattern: "/v1/help/", + Path: "/v1/apple/", + Expected: false, + }, + { + Pattern: "/v1/help/", + Path: "/v1/banana/", + Expected: false, + }, + { + Pattern: "/v1/help/", + Path: "/v1/cherry/", + Expected: false, + }, + + + + { + Pattern: "/v1/user/{user_id}", + Path: "/v1/user/123", + Expected: true, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "/v1/user/123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "//v1/user/123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "/v1//user/123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "/v1/user//123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "//v1//user/123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "/v1//user//123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "//v1//user//123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}", + Path: "//v1//user//123//", + Expected: false, + }, + + + + { + Pattern: "/v1/user/{user_id}/", + Path: "/v1/user/123/", + Expected: true, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "/v1/user/123", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "//v1/user/123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "/v1//user/123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "/v1/user//123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "//v1//user/123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "/v1//user//123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "//v1//user//123/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/", + Path: "//v1//user//123//", + Expected: false, + }, + + + { Pattern: "/v1/user/{user_id}/contact/{contact_type}", Path: "/v1/user/123/contact/e-mail", Expected: true, }, + { + Pattern: "/v1/user/{user_id}/contact/{contact_type}", + Path: "/v1/user/123/contact/e-mail/", + Expected: false, + }, { Pattern: "/v1/user/{user_id}/contact/{contact_type}", Path: "/v2/user/123/contact/e-mail", @@ -63,11 +207,39 @@ func TestPatternMatch(t *testing.T) { + { + Pattern: "/v1/user/{user_id}/contact/{contact_type}/", + Path: "/v1/user/123/contact/e-mail/", + Expected: true, + }, + { + Pattern: "/v1/user/{user_id}/contact/{contact_type}/", + Path: "/v1/user/123/contact/e-mail", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/contact/{contact_type}/", + Path: "/v2/user/123/contact/e-mail/", + Expected: false, + }, + { + Pattern: "/v1/user/{user_id}/contact/{contact_type}/", + Path: "/v2/user/123/contact/e-mail/", + Expected: false, + }, + + + { Pattern: "/v1/company/{company_name}", Path: "/v1/company/acme", Expected: true, }, + { + Pattern: "/v1/company/{company_name}", + Path: "/v1/company/acme/", + Expected: false, + }, { Pattern: "/v1/company/{company_name}", Path: "/v2/company/acme", @@ -83,6 +255,34 @@ func TestPatternMatch(t *testing.T) { Path: "/v1/COMPANY/acme", Expected: false, }, + + + + { + Pattern: "/v1/company/{company_name}/", + Path: "/v1/company/acme/", + Expected: true, + }, + { + Pattern: "/v1/company/{company_name}/", + Path: "/v1/company/acme", + Expected: false, + }, + { + Pattern: "/v1/company/{company_name}/", + Path: "/v2/company/acme/", + Expected: false, + }, + { + Pattern: "/v1/company/{company_name}/", + Path: "/v1/user/acme/", + Expected: false, + }, + { + Pattern: "/v1/company/{company_name}/", + Path: "/v1/COMPANY/acme/", + Expected: false, + }, } for testNumber, test := range tests { @@ -90,17 +290,26 @@ func TestPatternMatch(t *testing.T) { if err := pathmatch.CompileTo(&pattern, test.Pattern); nil != err { t.Errorf("For test #%d, did not expect an error, but actually got one: (%T) %q", testNumber, err, err) + t.Errorf("\t: PATTERN: %q", test.Pattern) + t.Errorf("\t: PATH: %q", test.Path) + t.Errorf("\t: EXPECTED: %t", test.Expected) continue } matched, err := pattern.Match(test.Path) if nil != err { t.Errorf("For test #%d, did not expect an error, but actually got one: (%T) %q", testNumber, err, err) + t.Errorf("\t: PATTERN: %q", test.Pattern) + t.Errorf("\t: PATH: %q", test.Path) + t.Errorf("\t: EXPECTED: %t", test.Expected) continue } if expected, actual := test.Expected, matched; expected != actual { t.Errorf("For test #%d, expected %t, but actually got %t.", testNumber, expected, actual) + t.Errorf("\t: PATTERN: %q", test.Pattern) + t.Errorf("\t: PATH: %q", test.Path) + t.Errorf("\t: EXPECTED: %t", test.Expected) continue } }