fix: preserve a req.url rewritten inside a mounted layer - #206
Conversation
When a layer mounted with a path is entered and nothing is left of the path, the router injects a leading slash into req.url. That slash used to be removed unconditionally on the way out, so when the layer rewrote req.url the real leading slash was stripped and the mount path was restored without a separator (/app + /index.html became /appindex.html). Remember the exact URL the injected slash produced and only strip it again when the layer left req.url untouched. Ref: expressjs/express#4059
Regression tests for the mount path being restored without a separator when a layer rewrote req.url, including the case where the original url carried a query string.
The stripped mount path is only mentioned as being removed, not as being added back, which makes the behavior of rewriting req.url inside a mounted middleware unclear.
|
CI here is waiting on maintainer approval, so I ran the |
kilisamemarisaaa
left a comment
There was a problem hiding this comment.
Reviewed commit 1b53c8a.\n\nI found no blocking issues in the changed behavior. Locally on Windows with Node v24.12.0:\n-
pm test: 696 passing\n-
pm run lint: passed\n- exercised untouched /foo, /foo?x=1, rewritten /bar, query-only rewrites, and nested mounted routers; the injected slash is removed only when the middleware leaves the synthetic URL unchanged, and rewritten paths retain the mount separator as intended.\n\nThe two regression tests also cover the reported no-trailing-slash case with and without an original query string.
Fixes the
req.urlrewriting bug reported in expressjs/express#4059.Problem
When a layer mounted with a path is entered and there is nothing left of the path, the router injects a leading slash so that
req.urlis never an empty string. On the way back out that slash was removed unconditionally, even when the layer had replacedreq.urlin the meantime, so the mount path was restored without a separator:GET /foo(no trailing slash) is the trigger; withGET /foo/there is nothing to strip and the rewrite works as expected. This is what breaks rewriting middleware such asconnect-history-api-fallbackwhen it is mounted on a path.Fix
Remember the exact url the injected slash produced and only strip it again when the layer left
req.urluntouched. Comparing against the stored value rather than against'/'also keeps it correct when a query string is present, since the injected value is then'/?...'rather than'/'.Tests
Two regression tests under
.use(path, ...fn)>req.url, one plain and one where the original url carried a query string. The existing strip/restore tests are untouched.Docs
Added a note to the
router.use([path], ...middleware)section: the stripped path is added back when the middleware callsnext(), and a rewrittenreq.urlis treated as relative to the mount path.