Repository navigation
fix: support multi-segment base paths in use() - #227
Closed
afonsojanu wants to merge 1 commit into
Closed
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
app.use('/foo/bar', handler) never actually matched incoming requests
because the request path was truncated down to just its first segment
before being looked up against the (correctly, full-path-keyed) bwares
and apps tables, so anything mounted on a base with more than one
"folder" silently 404'd.
Replaces that truncated lookup with a small helper that scans the
registered base keys and picks the longest one that's a full-segment
prefix of the request path, so both middleware bases and sub-app
mounts work regardless of how many segments they have. Single-segment
bases keep working exactly as before.
Author
|
Closing this for now as part of a clean slate — stepping back from everything I had open across these repos. |
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.
Closes #217.
app.use('/foo/bar', handler)never actually gets hit, even for requests that should match it — it always 404s. Here's the repro from the issue:The cause is in
handler(): the incoming request path is truncated down to just its first "folder" (viavalue()) before being looked up inbwares/apps, but those tables are keyed by the full base string passed touse(). So/foo/bar/main.jsgets reduced to/foofor the lookup, which never matches the/foo/barkey it was actually stored under, and the middleware/sub-app silently never runs. Single-segment bases (/baz) happen to work only because there's nothing to truncate.This adds a small
matchBase()helper that scans the registered keys and picks the longest one that's a full path-segment prefix of the request path (so/foowon't accidentally match a request for/foobar), and uses it for both thebwaresandappslookups instead of the single truncated segment. Single-segment bases keep behaving exactly as before.Added two tests: one covering a multi-segment middleware base (plus making sure a sibling single-segment path under the same first folder isn't affected), and one covering
use(base, subApp)with a multi-segment base.Ran the full existing suite before and after with
git stash— same single pre-existing failure both times (polka/url :: usage — throws if no input, unrelated to this change, looks like a Node version difference in theurl.parse()deprecation path). Everything else, including the 2 new tests, passes clean.