Repository navigation
Conversation
globMatch used minimatch with its defaults: `*` and `**` skip dot-files, `*` never matches an empty segment, a leading `!` negates the pattern, extglobs work and a trailing slash is ignored. Go Casbin uses doublestar, which does none of that, so a globMatch deny rule like `/admin/*` let `/admin/.env` and `/admin/` through in Node only. Now the pattern is turned into a RegExp with doublestar rules (`*`, `?`, `**` segments, classes, braces, `\\` escapes); a bad pattern matches nothing. minimatch is no longer used, so drop the dependency. Fixes #582
The RegExp built for `**` backtracked: a pattern like /**/a/**/a/**/y took seconds on a 6000-character path. Expand braces first, then match tokens with a dynamic program over (token, position), which takes O(tokens * length) time. `**` now also matches line breaks, like in Go.
Member
|
see: #582 (comment) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #582
globMatchused minimatch with its default options, while Go Casbin'sutil.GlobMatchuses doublestar. They differ a lot on paths:*/**skip dot-files:/admin/.envdid not match/admin/*,/files/.git/configdid not match/files/***never matched an empty segment:/admin/did not match/admin/*!negated the whole pattern:/a/bmatched!/c/a/b/matched/a/b+(b)workedSo a
globMatchdeny rule (p, guest, /admin/*, read, deny) let/admin/.envand/admin/through in Node, while Go denies them.minimatch has no option for the empty-segment and trailing-slash cases, so this PR adds a small matcher with doublestar's rules instead:
*(any non-/run, also empty and dot-files),?,**as a whole segment (zero or more directories;/a/**also matches/a),[...]/[!...]/[^...],{a,b}and\escapes. Braces are expanded first (max 1024 alternatives), then the tokens are matched with a small dynamic program over (token, position), so it runs in O(tokens × length) and never backtracks:/**/a/**/a/**/a/**/yagainst a 6000-character path takes ~1 ms (minimatch needs minutes there). A bad pattern (/[a,/{a, trailing\) matches nothing; Go returns an error there. The old*/workaround for minimatch is not needed anymore. minimatch is no longer used, so I removed it fromdependencies(and its 3 entries fromyarn.lock;yarn install --frozen-lockfilepasses).Testing
globMatchcases intest/util.test.tsfail before (/admin/.envvs/admin/*) and pass after. All 71 literalglobMatchexpectations in the file (old + new, incl. keys with\n) give the same result in Go Casbin v2.135.0util.GlobMatch. A long-path test guards against backtracking.falsefor/admin/users,/admin/.envand/admin/, same as Go.util.GlobMatchon 120,000 key/pattern pairs (path-like and random characters): old code differs on 844, new code on 94. The 94 left are odd patterns where doublestar has its own quirks: 3 or more*in a row, empty{}alternatives ({a,}),**inside braces (/avs/a/{**,x}is false in Go, but/avs/a/**is true), and some empty matches before/**(/vs/*/**is false in Go). Happy to handle any of those if you want exact parity there.jest(17 suites, 239 passed),eslint . --ext .js,.ts, prettier,npm run buildclean.