Make the {id} regexp less strict for identifying the "path" label value. (#2034)
Previously we were matching what boundary considers an id, but that is determined after requests are routed by grpc-gateway.
This changes the regexp to match any path segment to try to behave the same way that grpc-gateway would when assigning values
to the {id} variable declared in the protobuf.
This commit is contained in:
2 files changed
+26
-10
No files matched your search
@@ -91,9 +91,10 @@ func apiPathsAndMethods() map[string][]string {
|
||||
}
|
||||
|
||||
func buildRegexFromPath(p string) *regexp.Regexp {
|
||||
// a public id in boundary consists of a some prefix, an underscore and
|
||||
// at least 10 alphanumerical characters like "h_1234567890".
|
||||
const idRegexp = "[[:alnum:]]{1,}_[[:alnum:]]{10,}"
|
||||
// We only care about how grpc-gateway will route to specific handlers.
|
||||
// As long as there is at least 1 character that is part of a path segment
|
||||
// (not a '/', '?', or ':' we have identified an id for the sake of routing.
|
||||
const idRegexp = "[^\\/\\?\\:]+"
|
||||
|
||||
// Replace any tag in the form of {id} or {auth_method_id} with the above
|
||||
// regex so we can match paths to that when measuring requests.
|
||||
|
||||
@@ -20,13 +20,15 @@ func TestBuildRegexFromPath(t *testing.T) {
|
||||
match: []string{
|
||||
"/v1/pathsomething/a4s_aKsdFh723018djsa:test-thing",
|
||||
"/v1/pathsomething/h_1234567890:test-thing",
|
||||
},
|
||||
dont: []string{
|
||||
"/v1/pathsomething/{id}:test-thing",
|
||||
"/v1/pathsomething/{any_old_id}:test-thing",
|
||||
"/v1/pathsomething:test-thing",
|
||||
"/v1/pathsomething/not-an-id:test-thing",
|
||||
},
|
||||
dont: []string{
|
||||
"/v1/pathsomething/a4s_aKsdFh723018djsa:other-thing",
|
||||
"/v1/pathsomething:test-thing",
|
||||
"/v1/pathsomething/:test-thing:test-thing",
|
||||
"/v1/pathsomething/:other-thing",
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -34,11 +36,16 @@ func TestBuildRegexFromPath(t *testing.T) {
|
||||
match: []string{
|
||||
"/v1/pathsomething/a4s_aKsdFh723018djsa:authenticate",
|
||||
"/v1/pathsomething/am_1234567890:authenticate",
|
||||
},
|
||||
dont: []string{
|
||||
"/v1/pathsomething/{id}:authenticate",
|
||||
"/v1/pathsomething/{auth_method}:authenticate",
|
||||
},
|
||||
dont: []string{
|
||||
"/v1/pathsomething:authenticate",
|
||||
"/v1/pathsomething:authenticate:authenticate",
|
||||
"/v1/pathsomething/:authenticate:authenticate",
|
||||
"/v1/pathsomething/am_1234567890/:authenticate",
|
||||
"/v1/pathsomething/?whatabout=:authenticate",
|
||||
},
|
||||
},
|
||||
{
|
||||
path: "/v1/pathsomething/{non_id_tag}:test",
|
||||
@@ -66,10 +73,10 @@ func TestBuildRegexFromPath(t *testing.T) {
|
||||
t.Run(tc.path, func(t *testing.T) {
|
||||
r := buildRegexFromPath(tc.path)
|
||||
for _, m := range tc.match {
|
||||
assert.True(t, r.Match([]byte(m)))
|
||||
assert.True(t, r.Match([]byte(m)), "Couldn't match %q", m)
|
||||
}
|
||||
for _, d := range tc.dont {
|
||||
assert.False(t, r.Match([]byte(d)))
|
||||
assert.False(t, r.Match([]byte(d)), "Matched %q", d)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -100,6 +107,14 @@ func TestPathLabel(t *testing.T) {
|
||||
in: "/v1/accounts/a_1234567890",
|
||||
want: "/v1/accounts/{id}",
|
||||
},
|
||||
{
|
||||
in: "/v1/accounts/mistake",
|
||||
want: "/v1/accounts/{id}",
|
||||
},
|
||||
{
|
||||
in: "/v1/accounts/mistyped_id",
|
||||
want: "/v1/accounts/{id}",
|
||||
},
|
||||
{
|
||||
in: "v1/accounts/a_1234567890",
|
||||
want: "/v1/accounts/{id}",
|
||||
|
||||
Reference in new issue
Block a user