Be strict(er) in matching of last path component - #146
Open
awagner-mainz wants to merge 4 commits into
Open
Conversation
router:create-regex built path-matching regexes anchored only at the start (^/api/reconcile), never at the end. Since fn:matches() performs substring matching unless explicitly end-anchored, any path merely starting with a declared route matched it - e.g. GET /api/reconcile/v0.2 or /api/reconcile/foo/bar both matched the bare /api/reconcile route and returned its manifest instead of a 404. Same defect affects parameterized routes too, just usually masked by specificity-based route selection when a more specific, correctly-matching route also exists for the same request. This is a known, previously-unfixed upstream issue (eeditiones#122, "Matching rule with superfluous chars") - the exact same bug, reported independently there in June with a FHIR-style /Patient example. Fix: append the missing "$" to the returned regex. Verified against a running instance: the bug case (garbage sub-paths under a real route) no longer returns the wrong handler's response, while every legitimately declared route (including parameterized ones) still matches correctly. Full XQSuite + Cypress regression suites for the reconcile profile stay green, and other apps sharing this library were spot-checked for breakage.
The end-anchor fix in 910f131 (eeditiones#122) correctly stopped a route pattern from over-matching its own sub-paths, but as a side effect also stopped it matching its own trailing-slash variant: "POST /api/reconcile/" started 405ing where the old, unanchored regex had tolerated it "by accident". Anchor with an optional single trailing slash ("/?$") instead of a bare "$" - still rejects deeper sub-paths like "/api/reconcile/v0.2", just also accepts the bare trailing slash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…the fix's comment Points at the downstream mpilhlt/tei-publisher-reconcile project's docker/README.md for the full repro and regression coverage, so the comment stays useful without needing to dig through the original fix commits' messages.
Updated comments to clarify routing behavior and restrictions.
awagner-mainz
changed the title
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
1 participant
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.
My testsuite for https://github.com/mpilhlt/tei-publisher-reconcile has turned up some errors, one of them concerning roaster. It seems the matching of path parameters would allow any last component that had the required string as a prefix to match. See also #122. This PR should fix this (while allowing a trailing slash).
For the testsuite and description of the context in which I encountered this, see https://github.com/mpilhlt/tei-publisher-reconcile/blob/main/docker/README.md#verified-end-to-end