Fix duplicated query string and hash in bare-domain links - #72
Open
hyldmo wants to merge 1 commit into
Open
Conversation
When a URL's host is followed directly by `?` or `#` with no path (e.g. `https://example.com?foo=bar`), `origin` is parsed via `href.split('/', 3).join('/')`, which assumes a `scheme://host/...` shape and absorbs the query/hash into `origin`. The bare-domain branch then appends `url.search`/`url.hash` again, producing a duplicated query/hash in the shortened text: https://example.com?foo=bar -> example.com?foo=bar?foo=bar https://example.com#frag -> example.com#frag#frag A trailing slash (`example.com/?foo=bar`) hid the bug, so no existing fixture exercised the slash-less case. Strip any `?query`/`#hash` from `origin` so it stays `scheme://host`. The replace is a no-op for URLs that have a path (every prior case), so existing snapshots are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
The bug
When a URL's host is followed directly by
?or#with no path, the shortened text repeats the query string / hash:https://example.com?foo=barexample.com?foo=bar?foo=barexample.com?foo=barhttps://example.com#fragexample.com#frag#fragexample.com#fragThis is visible in the wild via Refined GitHub's
shorten-linksfeature: a comment containing a bare-domain link with a query string (e.g. a CI preview-environment link likehttps://sub.example.com?preview_id=123) renders its link text with the query string twice, while thehrefstays correct.Root cause
originis parsed manually to avoid encoding/punycode issues:This assumes a
scheme://host/…shape. When there is no/between the host and the?/#, the query/hash is absorbed intoorigin. The bare-domain branch (pathname === '/') then appendsurl.search/decodeURI(url.hash)a second time, producing the duplication.A trailing slash (
example.com/?foo=bar) makessplit('/', 3)stop at the host, so the bug only triggers on the slash-less form — which is why no existing fixture caught it.The fix
Strip any
?query/#hashfromoriginso it staysscheme://host[:port]:The
replaceis a no-op for any URL that has a path (i.e. every previously-handled case), so all existing snapshots are unchanged — the snapshot diff is purely additive.Tests
Added bare-domain regression fixtures covering query + hash, across
https(scheme stripped),http(scheme retained),www, and the already-working trailing-slash variant.npm test(xo + tsd + vitest, 141 tests) passes.