fix(history/html5): avoid SecurityError on documents with userinfo URL - #2726
fix(history/html5): avoid SecurityError on documents with userinfo URL#2726spokodev wants to merge 2 commits into
Conversation
When a Vue 3 app boots behind HTTP basic auth, the document URL carries userinfo (http://user:pass@host/). The existing createWebHistory() initial replaceState builds an absolute URL via location.protocol + '//' + location.host + base + to, and the browser then sees that URL as cross-origin against the document URL and throws SecurityError. vue-router catches the error and falls back to location.replace(url), which performs a full reload, so the user-facing symptom is "the app reloads itself on every router push" and any subsequent client-side navigation also reloads. See vuejs#2714. Stripping the host from the constructed URL when the document URL has userinfo lets pushState resolve the path against the current document origin, which avoids the cross-origin check entirely. The existing // protocol-relative escape hatch still routes through createBaseLocation() so '/foo' vs '//foo' keep their current semantics. The behavior change is scoped to documents whose URL has userinfo (detected via new URL(location.href).username because happy-dom does not implement location.username). All other code paths keep prepending location.protocol + '//' + location.host as before, so the existing 'prepends the host to support // urls' assertion and the file:// hash-base tests are unaffected. Tests cover: - positive: with userinfo URL, push('/foo') passes '/foo' to pushState instead of 'http://localhost:3000/foo' - edge: with userinfo URL, push('//foo') still resolves to 'http://localhost:3000//foo' so // urls are not broken - regression guard: with a plain document URL push('/foo') still yields 'http://localhost:3000/foo' - regression guard: file:// document URL push('/foo') still yields 'file:///foo' Fixes vuejs#2714.
✅ 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesSecurityError fix for userinfo URLs in
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/router/__tests__/history/html5.spec.ts (1)
109-123: ⚡ Quick winAdd a direct assertion for the initialization
replaceStatepath.The original failure happened during
createWebHistory()initialization, but this test currently starts spying after that call. Adding a pre-initreplaceStateassertion would pin the regression at the root trigger.Proposed test adjustment
it('passes a relative URL to push/replaceState to avoid SecurityError', () => { getWindow().happyDOM.setURL('http://test:test@localhost:3000/') + const replaceSpy = vi.spyOn(window.history, 'replaceState') let history = createWebHistory() + expect(replaceSpy).toHaveBeenCalledWith( + expect.anything(), + expect.any(String), + '/' + ) + replaceSpy.mockRestore() let spy = vi.spyOn(window.history, 'pushState') history.push('/foo')🤖 Prompt for 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. In `@packages/router/__tests__/history/html5.spec.ts` around lines 109 - 123, The test for the relative URL handling needs to verify that `replaceState` is called with a relative URL during `createWebHistory()` initialization, not just after. Move the spy setup for `window.history.replaceState` (and possibly `window.history.pushState`) to occur before the `createWebHistory()` call in this test function, then add an assertion to verify that the `replaceState` spy was called with a relative URL during initialization. This will catch the regression at its root trigger point rather than only checking the subsequent `push()` call.
🤖 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.
Nitpick comments:
In `@packages/router/__tests__/history/html5.spec.ts`:
- Around line 109-123: The test for the relative URL handling needs to verify
that `replaceState` is called with a relative URL during `createWebHistory()`
initialization, not just after. Move the spy setup for
`window.history.replaceState` (and possibly `window.history.pushState`) to occur
before the `createWebHistory()` call in this test function, then add an
assertion to verify that the `replaceState` spy was called with a relative URL
during initialization. This will catch the regression at its root trigger point
rather than only checking the subsequent `push()` call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8e3cc84-c956-4ebe-a2f6-97a9dd61199b
📒 Files selected for processing (2)
packages/router/__tests__/history/html5.spec.tspackages/router/src/history/html5.ts
The original SecurityError in vuejs#2714 was thrown by createWebHistory() during initialization, before any navigation. The existing tests start spying after that call, so they cover push but not the init path. Spy on replaceState before createWebHistory() and assert it receives a relative URL, pinning the regression at its real trigger.
When a Vue 3 app boots behind HTTP basic auth, the document URL carries userinfo (http://user:pass@host/`). The initial replaceState from createWebHistory() builds an absolute URL via location.protocol + '//' + location.host + base + to, and the browser sees that URL as cross-origin against the userinfo-bearing document and throws SecurityError. vue-router catches it and falls back to location.replace(url), which does a full reload, so the user-facing symptom is "the app reloads itself on every router push" and every subsequent client-side navigation also reloads.
Repro and full DevTools traces are in #2714. Posva's note on the issue suggested using createBaseLocation() only for file:// and an absolute (path-relative) URL for the others. The patch implements that suggestion in a slightly narrower form: it only drops the host prefix when the document URL has userinfo, so behaviour on the default code path is unchanged.
Change
In pushState inside packages/router/src/history/html5.ts, the URL constructed for non-hash bases is now path-relative when the document URL has userinfo. createBaseLocation() is still used for protocol-relative paths (//foo) so the existing 'prepends the host to support // urls' behaviour is preserved.
new URL(location.href).username is used instead of location.username because happy-dom (used by the test environment) does not implement location.username yet; the standard URL parser does, and the difference is invisible in real browsers.
Tests
Four cases under a new document URL with userinfo (HTTP basic auth) describe block in packages/router/tests/history/html5.spec.ts:
Verification
pnpm vitest run tests/history/html5.spec.ts -> 19 passed (15 existing + 4 new).
pnpm vitest run tests -> 682 passed, 3 skipped, 2 todo (no regressions).
pnpm lint clean on the changed files.
(pnpm run test:types requires a prior build and fails the same way on main without one, so I have not gated on it locally - it is the standard test-dts/experimental.test-d.ts "Cannot find module 'vue-router'" infra issue.)
Fixes #2714.
Summary by CodeRabbit
SecurityErrorthat could occur when navigating with URLs that include HTTP basic-auth credentials. History navigation now correctly uses safe, relative paths for those document URL formats.pushState/replaceStaterelative path behavior and unchanged handling for protocol-relative and file URL cases.