there was a bug in pathmatch.Find(). it allowed extra stuff at ne, and basically only matched beginning of pattern. correct it.

master
Charles Iliya Krempeaux 2019-06-24 14:17:57 -07:00
parent 9a424db462
commit ebbb49df89
2 changed files with 232 additions and 13 deletions

View File

@ -12,6 +12,11 @@ var (
errThisShouldNeverHappen = newInternalError("This should never happen.") 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) { func (pattern *Pattern) Find(path string, args ...interface{}) (bool, error) {
if nil == pattern { if nil == pattern {
return false, errNilReceiver return false, errNilReceiver
@ -34,23 +39,28 @@ func (pattern *Pattern) Find(path string, args ...interface{}) (bool, error) {
s = s[len(bit):] s = s[len(bit):]
case wildcardBit: case wildcardBit:
index := strings.IndexRune(s, '/') index := strings.IndexRune(s, '/')
if -1 == index {
if err := set(s, argsIndex, args...); nil != err { var value string
return doesNotMatter, err switch {
} default:
argsIndex++
} else if 0 <= index {
value := s[:index]
if err := set(value, argsIndex, args...); nil != err {
return doesNotMatter, err
}
argsIndex++
s = s[index:]
} else {
return doesNotMatter, errThisShouldNeverHappen 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 return true, nil
} }

View File

@ -17,6 +17,11 @@ func TestPatternMatch(t *testing.T) {
Path: "/v1/help", Path: "/v1/help",
Expected: true, Expected: true,
}, },
{
Pattern: "/v1/help",
Path: "/v1/help/",
Expected: false,
},
{ {
Pattern: "/v1/help", Pattern: "/v1/help",
Path: "/v2/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}", Pattern: "/v1/user/{user_id}/contact/{contact_type}",
Path: "/v1/user/123/contact/e-mail", Path: "/v1/user/123/contact/e-mail",
Expected: true, 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}", Pattern: "/v1/user/{user_id}/contact/{contact_type}",
Path: "/v2/user/123/contact/e-mail", 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}", Pattern: "/v1/company/{company_name}",
Path: "/v1/company/acme", Path: "/v1/company/acme",
Expected: true, Expected: true,
}, },
{
Pattern: "/v1/company/{company_name}",
Path: "/v1/company/acme/",
Expected: false,
},
{ {
Pattern: "/v1/company/{company_name}", Pattern: "/v1/company/{company_name}",
Path: "/v2/company/acme", Path: "/v2/company/acme",
@ -83,6 +255,34 @@ func TestPatternMatch(t *testing.T) {
Path: "/v1/COMPANY/acme", Path: "/v1/COMPANY/acme",
Expected: false, 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 { for testNumber, test := range tests {
@ -90,17 +290,26 @@ func TestPatternMatch(t *testing.T) {
if err := pathmatch.CompileTo(&pattern, test.Pattern); nil != err { 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("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 continue
} }
matched, err := pattern.Match(test.Path) matched, err := pattern.Match(test.Path)
if nil != err { if nil != err {
t.Errorf("For test #%d, did not expect an error, but actually got one: (%T) %q", testNumber, err, 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 continue
} }
if expected, actual := test.Expected, matched; expected != actual { if expected, actual := test.Expected, matched; expected != actual {
t.Errorf("For test #%d, expected %t, but actually got %t.", testNumber, 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 continue
} }
} }