Repository navigation
DM-56095: Redirect /v/{edition} without trailing slash - #883
Merged
Merged
Conversation
A request for /v/{edition} (no trailing slash) served the edition's
index.html in place with a 200 instead of redirecting to /v/{edition}/.
The browser then resolved the page's relative asset links against /v/
rather than /v/{edition}/, so Sphinx CSS and JS failed to load and the
page rendered unstyled. Reported against pipelines.lsst.io/v/weekly.
The resolver's directory-index fallback only redirected when route.path
was non-empty, a guard meant to stop the main edition's "/" from
redirecting to "//". But the edition root has an empty route.path for
both /v/weekly/ and /v/weekly, so the no-slash form slipped through.
Decide from the request pathname instead: redirect whenever the index
file is found and the pathname does not already end in "/". This also
covers the path-prefix scheme's /docs/{project} root.
Add resolver unit tests for the edition root with and without the slash
(including query-string preservation) and integration tests for the
subdomain and path-prefix schemes.
jonathansick
force-pushed
the
tickets/DM-56095-edition-root-redirect
branch
from
November 9, 2026 19:19
7b7c73e to
98899c7
Compare
The directory-index fallback built its own URL and Response.redirect, duplicating the resolveRedirect helper that the /v redirect route already uses. Route both canonicalising redirects through the helper so they cannot drift apart.
Response.redirect() emits a bare 301 with only a Location header. The Cloudflare edge cannot cache a response without a cache directive, so every no-slash request reached the Worker and paid KV + R2 I/O, while browsers treat a header-less 301 as heuristically cacheable forever. Build the 301 by hand in resolveRedirect and attach CACHE_CONTROL_LONG (5 min browser, 1 day edge). The target is a pure function of the request URL's shape, never of edition content, so the long profile is safe regardless of the edition's own cache_profile. Assert the header in the redirect unit and integration tests.
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.
Summary
/v/{edition}with no trailing slash served the edition'sindex.htmlin place with a 200 instead of redirecting to/v/{edition}/. The browser then resolved the page's relative asset links against/v/rather than/v/{edition}/, so Sphinx CSS and JS failed to load and the page rendered unstyled. Reported by a user againsthttps://pipelines.lsst.io/v/weekly.route.pathwas non-empty, a guard meant to stop the main edition's/from redirecting to//. The edition root has an emptyroute.pathfor both/v/weekly/and/v/weekly, so the no-slash form slipped through. The guard now checks the request pathname instead, which also covers the path-prefix scheme's/docs/{project}root.Validation steps
production-rubinwithdocverse deploy-worker(note: the prod worker has not been redeployed since mid-September, so this deploy also picks up the later worker commits onmain, including thecache_profileCache-Control change).curl -sI https://pipelines.lsst.io/v/weeklyreturns301withLocation: https://pipelines.lsst.io/v/weekly/.curl -sI https://pipelines.lsst.io/v/weekly/still returns200.curl -sI https://pipelines.lsst.io/still returns200(no//redirect loop on the main edition root).curl -sI https://pipelines.lsst.io/v/weekly/getting-startedstill returns301to the slash-terminated URL.https://pipelines.lsst.io/v/weeklyin a browser and confirm the page is styled.References
🤖 Generated with Claude Code