fix(history): use absolute path on http(s) to keep userinfo#2717
fix(history): use absolute path on http(s) to keep userinfo#2717gluebi wants to merge 2 commits into
Conversation
✅ Deploy Preview for vue-router canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR changes HTML5 history URL construction: changeLocation now conditionally prefixes origins only for ChangesHTML5 History URL Construction Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4f78e11 to
4037491
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/router/src/history/html5.ts`:
- Around line 235-236: The protocol-relative ("//") path handling in html5.ts
currently rebuilds the URL using location.host which strips userinfo; update the
branch that handles (base + to).startsWith('//') so it constructs the
protocol-relative prefix from the current document URL in a way that preserves
userinfo (e.g., derive the "scheme + // + authority" portion from location.href
or createBaseLocation() that includes userinfo instead of using location.host),
adjust the logic inside the same function that handles history.push to use this
preserved-authority prefix for protocol-relative targets, and add a regression
test exercising history.push('//foo') on a credentialed document URL to ensure
userinfo is retained and the same SecurityError behavior is reproduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bbb9df8d-bfc1-46f9-9eda-8d771617fbf7
📒 Files selected for processing (2)
packages/router/__tests__/history/html5.spec.tspackages/router/src/history/html5.ts
Chromium enforces userinfo equality on pushState/replaceState. Building the URL via `location.protocol + '//' + location.host` strips userinfo and trips SecurityError on basic-auth documents like http://user:pass@host/. Pass an absolute path on http(s) so the browser resolves it against the document URL. For file:// (no host) and `//`-prefixed paths (vuejs#261), still use createBaseLocation() but include userinfo so those branches don't strip it either. Refs vuejs#2714
4037491 to
2d0dabe
Compare
Refs #2714
Problem
On Chromium, when the document URL contains userinfo (e.g.
http://user:pass@host/from an HTTP Basic-Auth challenge),createWebHistory()'s initialhistory.replaceStatethrowsSecurityError. Chromium enforces userinfo equality onpushState/replaceState, not just origin equality.changeLocationbuilds the URL viacreateBaseLocation() + base + to, wherecreateBaseLocation = () => location.protocol + '//' + location.host.location.hostishostname[:port]by spec, so userinfo is dropped.The resulting
replaceStateURLhttp://host/mismatches thedocument URL
http://user:pass@host/→SecurityError→ thesurrounding try/catch falls back to
location.replace(url), whichsilently reloads the page to the userinfo-less URL on the user's
first basic-auth visit, losing client-side state.
Live repro (Chromium, docker compose + nginx basic-auth):
https://github.com/gluebi/vue-router-5-basic-auth-repro
Fix
Per your hint in #2714, skip
createBaseLocation()for non-file:protocols. Pass an absolute path (
base + to) and let the browserresolve it against the document URL — that preserves scheme +
userinfo + host + port automatically.
For the remaining branches (
file://and protocol-relative//paths),
createBaseLocation()is still used. To keep userinfo thereas well,
createBaseLocation()now reads it fromlocation.hrefvia the URL constructor (since
location.hoststrips it). file://is unaffected in practice (no userinfo), but
push('//foo')on acredentialed document URL no longer hits the same
SecurityError.The protocol-relative
//branch is preserved so the defense from#261 isn't regressed.
Tests
passes an absolute path when document URL contains userinforegression for createWebHistory() initial replaceState throws SecurityError when document URL contains userinfo (HTTP basic-auth in URL) #2714 (happy-dom
setURLsimulates userinfo).keeps userinfo when prepending the host for // urlscoversthe
createBaseLocation()userinfo path (push('//foo') on abasic-auth URL retains
test:test@).prepends the file:// origin on file documents without a hash basecovers the
location.protocol === 'file:'branch (previously nofile:// test ran without a hash base).
prepends the host to support // urlsupdated —/foonow asserts an absolute path; the
//foodefense path is unchanged.Summary by CodeRabbit
Bug Fixes
Tests