Repository navigation
fix: never emit a double slash in page fullpaths - #263
Open
erdostom-bot wants to merge 1 commit into
Open
erdostom-bot wants to merge 1 commit into
erdostom-bot wants to merge 1 commit into
Conversation
GetPageFullpath#build_fullpath joined the base url, the locale prefix and
the page path blindly, so a path stored with a leading slash
("/hello-world"), a base url ending with a slash or a static page path
declared as "/products" produced hrefs like "https://host//hello-world".
- GetPageFullpath.join strips leading/trailing slashes from every segment,
collapses inner "//" and always yields a single leading "/" when there
is no base url (the scheme's "//" is kept).
- The static page branch of TransformLinkConcern uses the same
normalisation (absolute urls are left untouched).
- PagePath#clean_value now strips every leading/trailing slash (it only
removed one) and collapses any run of slashes (it only collapsed 3+).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit e668f78)
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.
Problem
Link settings pointing to a Maglev page (or a static page) can render with a double slash, e.g.
https://example.com//hello-world.Maglev::GetPageFullpath#build_fullpathjoins[base_url, locale?, path]with'/'without looking at the pieces, and the static page branch ofGetPageSections::TransformLinkConcernreturns the configured path untouched. In live modebase_urlisrequest.base_url(https://host), so any slash already present on the path side becomeshost//path.When it happens
Maglev::PagePath#clean_valuestrips only one leading/trailing slash and only collapses runs of 3+ slashes, so//hellois stored as/helloanda//bas is. Paths written around validations (imports,update_column, rows predating the cleaner) keep their leading slash.config.static_pages): never normalised./productswas returned as/products,productsasproducts(a relative href).base_urlwith a trailing slash (customGetBaseUrl, engine mounted at/):base/+/path.[host, 'fr', '/path']giveshost/fr//path.Fix
Maglev::GetPageFullpath.join(base_url, *segments): trims leading/trailing slashes from every segment, collapses inner//, keeps the base url's scheme, and always returns a single leading/when there is no base url (/for the index page).build_fullpathuses it;/indexis treated likeindex./products); absolute urls are left untouched. Small behaviour change for static pages declared without a leading slash, consistent with the page branch.PagePath#clean_valuestrips every leading/trailing slash and collapses any//run.Tests
spec/services/maglev/get_page_fullpath_spec.rb: leading slash, base url with trailing slash (preview), host base url (live) incl. locale prefix and index, nil base url, static page with/hello-world/,.joinedge cases.spec/services/maglev/fetch_sections_content_service_spec.rb: static page link/products/→/products, absolute static url untouched, anchor href.spec/models/maglev/page_path_spec.rb: cleaning of the stored value.12 of the new examples fail on
mainwithout the fix; all pass with it. RuboCop clean; full non-system suite green locally (postgres).🤖 Generated with Claude Code