Skip to content

fix: read keyGet2 values from the matched capture groups - #584

Merged
hsluoyz merged 1 commit into
apache:masterfrom
bardrop-oss:fix-keyget2-capture-groups
Nov 15, 2026
Merged

hsluoyz merged 1 commit into
apache:masterfrom
bardrop-oss:fix-keyget2-capture-groups

Conversation

@bardrop

@bardrop bardrop commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #583

keyGet2 split key1 and key2 on / and returned the segment of key1 at the same index as :pathVar in key2. So:

  • /* that spans more than one segment shifted the index: keyGet2('/orgs/acme/teams/t1/docs/42', '/orgs/*/docs/:id', 'id') gave t1.
  • :pathVar inside a segment never matched (keyMatch2 only expands /:): keyGet2('/files/report-7', '/files/report-:id', 'id') gave "".

Now keyGet2 builds the regex the same way Go Casbin's util.KeyGet2 does (/* → /.*, :name → ([^/]+)) and returns the capture group of :pathVar. A bare * pattern still gives "" and does not throw.

Testing

  • New cases in test/util.test.ts fail before (Expected "42", Received "t1") and pass after.
  • Ran 22 inputs (the old tests, the issue cases, repeated / partial / multiple tokens, *) through Go Casbin v2.135.0 util.KeyGet2: the new code gives the same result for all 22, the old code differed on 4.
  • jest (17 suites, 239 passed), eslint . --ext .js,.ts, prettier, npm run build clean.

keyGet2 took the segment of key1 at the same split index as :pathVar in
key2. That is wrong when /* spans more than one segment, and it gives ""
when :pathVar is only part of a segment (/files/report-:id).

Now it builds the regex like Go Casbin's KeyGet2 (/* -> /.*, :name ->
([^/]+)) and returns the capture group of :pathVar.

Fixes #583
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@152ccd9). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #584   +/-   ##
=========================================
  Coverage          ?   71.40%           
=========================================
  Files             ?       34           
  Lines             ?     2207           
  Branches          ?      518           
=========================================
  Hits              ?     1576           
  Misses            ?      380           
  Partials          ?      251           
Flag Coverage Δ
node-^20 71.40% <100.00%> (?)
node-^22 71.40% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hsluoyz
hsluoyz merged commit 70eb3e2 into apache:master Nov 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

keyGet2 returns the wrong segment when the pattern contains * or an inline :param

3 participants