mirror of
https://github.com/fosrl/pangolin.git
synced 2026-05-06 08:56:45 -05:00
Rules not matching specific path #229
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/pangolin#229
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @scetu on GitHub (Apr 10, 2025).
Path rule is incorrectly matching path for
/livekit-jwt-service/sfu/get, in Debug logs it is vissible that it get's matched by rule/livekit*or*livekit*but Final result for this rules isFalseand goes down to the catch-all ruleDeny all *I have also other rules for this resource, and they are evauluated correctly, but only this path
/livekit-jwt-service/sfu/getis incorrectly evaluated.@grokdesigns commented on GitHub (Apr 11, 2025):
I'd like to look into this issue. Can you confirm if making your pattern
/livekit*/*/*results in a match? Can you also provide some examples of patterns/paths that are working for you?@scetu commented on GitHub (Apr 11, 2025):
I can confirm that
/livekit*/*/*rule is working correctly for:/livekit-jwt-service/sfu/livekit-jwt-service/sfu/getWorking patterns for
/_matrix/*or/.well-known/matrix/*from beggining are/_matrix/client/unstable/org.matrix.simplified_msc3575/sync/.well-known/matrix/server/_matrix/client/v3/rooms/!HHdgEA@grokdesigns commented on GitHub (Apr 11, 2025):
@scetu It seems the difference here is that you have a wildcard only in the first segment for the one you're having an issue with, whereas the others, you have a static root segment and the wildcard somewhere after that.
This means that your original rule would match /livekit-1 or /livekit-2 or any other root path that starts with livekit, but subsequent segments in the path will cause the match to fail. Your other examples having the wildcard after the / means they will match any number of subdirectories after the root segment. This actually means you could use /livekit*/* and it should also match.
That's kind of what I expected when I asked you to try the other pattern. I have some ideas on this that might make this easier to handle and more flexible, but for now maybe @miloschwartz or @oschwartz10612 can make the documentation more explicit on what will/won't match.
@scetu commented on GitHub (Apr 11, 2025):
@grokdesigns I have created https://github.com/fosrl/docs/pull/17 with explanation from your clarification, you are right that
/livekit*/*is sufficient and is matching any subsequent segments of URL.@grokdesigns commented on GitHub (Apr 15, 2025):
@miloschwartz @oschwartz10612 What are your thoughts on using something like picomatch for path matching? I saw the module was in the project, but didn't seem to be used anywhere. I have a mostly working build that converts existing rules to picomatch compatible as part of the migration and then uses picomatch in the isPathAllowed and related functions. A pretty big change, but allows quite a bit of flexibility in matching in my testing.
@miloschwartz commented on GitHub (Apr 15, 2025):
@grokdesigns This would be good to tackle and improve because our current matching is not great. I also found https://github.com/pillarjs/path-to-regexp which seems like it could maybe be good since it's geared specifically toward URL? Let me know what you think.
@grokdesigns commented on GitHub (Apr 15, 2025):
@miloschwartz I hadn't seen that one before, but going down the rabbit hole, I also found this: https://urlpattern.spec.whatwg.org/ | https://nodejs.org/api/url.html, which is newly supported in v23 of Node and seems almost tailormade for this type of use case. I'll try to look into this more this evening.
Another good reference that specifically talks about URL matching for middleware type use cases: https://docs.deno.com/api/web/~/URLPattern
@miloschwartz commented on GitHub (Apr 16, 2025):
Yeah those both look promising. Let me know what you find.