arshilkxwork opened a new pull request, #1767:
URL: https://github.com/apache/casbin/pull/1767
`KeyMatch4`, `KeyGet2` and `KeyGet3` read the value of the n-th pattern
variable from capture group n+1. That only holds when the pattern has no
capture groups of its own. With one, the indices shift:
- `KeyMatch4("/a/42", "/(a|b)/{id}")` panics with `KeyMatch4: number of
tokens is not equal to number of values`. `Enforce` recovers it and returns a
`panic:` error with a stack trace. When `util.KeyMatch4` is registered with
`AddNamedMatchingFunc`, role lookups run outside that recover: after adding
`g2, /(book|pen)/{id}/copy/{id}, book_group`,
`GetNamedImplicitRolesForUser("g2", "/book/1/copy/1")` panics the caller. This
is the same role-manager path 0a73027 covered for invalid patterns.
- `KeyGet2("/a/x/42", "/(a|b)/x/:id", "id")` returns `"a"` instead of
`"42"`, and `KeyGet3` does the same with `{id}`. No error is raised. A group
that comes after the variable (`/:id/(a|b)`) happens to work today.
Each pattern variable now compiles to a named group,
`(?P<casbinToken0>[^/]+)` and so on, and `compilePattern` stores the submatch
index of each one using `SubexpIndex`. `keyGet` and `keyMatch4` read values
through those indices. The token/value count check in `keyMatch4` is removed,
since the two can no longer disagree.
Parse errors from these three functions quote the rewritten regexp, so they
now show the group names. `TestKeyGet2Func` pins that text and is updated to ``
`^/(?P<casbinToken0>[^/]+)/(using$` ``.
`KeyGet2`/`KeyGet3` also switch from `FindAllStringSubmatch` to
`FindStringSubmatch`. The pattern is anchored, so only the first match was ever
used. On my machine `BenchmarkKeyGet2` goes from 171 to 134 ns/op (3 allocs to
1), `BenchmarkKeyGet3` from 217 to 200 ns/op, and `BenchmarkKeyMatch4` is
unchanged.
Testing:
- New cases in `TestKeyGet2`, `TestKeyGet3` and `TestKeyMatch4` with groups
before, between and after the variables. On master the KeyGet cases return the
wrong segment and `TestKeyMatch4` panics.
- New `TestPatternWithCaptureGroupInPolicy` registers `util.KeyMatch4` as
the g2 matching function with `/(book|pen)/{id}/copy/{id}` and checks `Enforce`
and `GetNamedImplicitRolesForUser`. On master `Enforce` returns the panic error
and `GetNamedImplicitRolesForUser` panics.
- `go test -race ./...` passes, and golangci-lint v2.15.0 reports no issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
