fix(theme/Link): support custom URI schemes in external URL detection#3304
fix(theme/Link): support custom URI schemes in external URL detection#3304
Conversation
Agent-Logs-Url: https://github.com/web-infra-dev/rspress/sessions/ec8b44ba-c56a-4204-92b0-af0ec10d8947 Co-authored-by: SoonIter <79413249+SoonIter@users.noreply.github.com>
Deploying rspress-v2 with
|
| Latest commit: |
d7fda2c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ed4bc1b3.rspress-v2.pages.dev |
| Branch Preview URL: | https://copilot-support-custom-uri-s.rspress-v2.pages.dev |
Agent-Logs-Url: https://github.com/web-infra-dev/rspress/sessions/ec8b44ba-c56a-4204-92b0-af0ec10d8947 Co-authored-by: SoonIter <79413249+SoonIter@users.noreply.github.com>
Rsdoctor Bundle Diff AnalysisFound 3 projects in monorepo, 1 project with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 webPath:
📦 Download Diff Report: web Bundle Diff Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
Pull request overview
Updates shared runtime URL utilities to correctly classify custom URI schemes (e.g. vscode://, jetbrains://) as external, preventing them from being rewritten/resolved as relative links in theme Link navigation and editLink handling.
Changes:
- Replace hardcoded external URL prefixes with scheme-based detection in
isExternalUrl(), while blockingjavascript:,data:, andfile:. - Add unit tests covering custom schemes, common schemes, relative/protocol-relative paths, and blocked schemes.
- Extend
normalizeHref()test coverage to ensure custom schemes are preserved.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/shared/src/runtime-utils/utils.ts | Switch isExternalUrl() to scheme-based detection with an unsafe-scheme denylist. |
| packages/shared/src/runtime-utils/utils.test.ts | Add tests for the new scheme-aware behavior (custom schemes + unsafe schemes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const UNSAFE_EXTERNAL_URL_SCHEME_REGEXP = /^(?:javascript|data|file):/i; | ||
|
|
||
| export function isExternalUrl(url = '') { | ||
| return ( |
There was a problem hiding this comment.
EXTERNAL_URL_SCHEME_REGEXP will treat Windows drive-letter paths like C:\foo / C:/foo as having a URI scheme (c:), so isExternalUrl() returns true and downstream code (e.g. link classification / addLeadingSlash) will treat local paths as external. Consider explicitly excluding drive-letter patterns (e.g. ^[a-zA-Z]:[\\/]) or otherwise narrowing the scheme detection to avoid this false positive.
| const UNSAFE_EXTERNAL_URL_SCHEME_REGEXP = /^(?:javascript|data|file):/i; | |
| export function isExternalUrl(url = '') { | |
| return ( | |
| const UNSAFE_EXTERNAL_URL_SCHEME_REGEXP = /^(?:javascript|data|file):/i; | |
| const WINDOWS_DRIVE_LETTER_PATH_REGEXP = /^[a-zA-Z]:[\\/]/; | |
| export function isExternalUrl(url = '') { | |
| return ( | |
| !WINDOWS_DRIVE_LETTER_PATH_REGEXP.test(url) && |
| const EXTERNAL_URL_SCHEME_REGEXP = /^[a-zA-Z][a-zA-Z\d+\-.]*:/; | ||
| const UNSAFE_EXTERNAL_URL_SCHEME_REGEXP = /^(?:javascript|data|file):/i; | ||
|
|
||
| export function isExternalUrl(url = '') { | ||
| return ( | ||
| url.startsWith('http://') || | ||
| url.startsWith('https://') || | ||
| url.startsWith('mailto:') || | ||
| url.startsWith('tel:') | ||
| EXTERNAL_URL_SCHEME_REGEXP.test(url) && | ||
| !UNSAFE_EXTERNAL_URL_SCHEME_REGEXP.test(url) | ||
| ); |
There was a problem hiding this comment.
isExternalUrl() doesn’t normalize leading whitespace before applying the scheme/unsafe-scheme regexes. Inputs like ' javascript:alert(1)' or '\n data:text/plain,...' won’t match UNSAFE_EXTERNAL_URL_SCHEME_REGEXP, and also won’t match the scheme regexp, leading to inconsistent handling vs isDataUrl() (which allows leading whitespace). Consider trimStart() (or using ^\s* in both regexes) so unsafe schemes are consistently recognized and blocked even when padded.
| expect(isExternalUrl('//example.com/foo')).toBe(false); | ||
| expect(isExternalUrl('file:///Users/test/docs/index.md')).toBe(false); | ||
| expect(isExternalUrl('javascript:alert(1)')).toBe(false); | ||
| expect(isExternalUrl('data:text/plain,hello')).toBe(false); |
There was a problem hiding this comment.
The isExternalUrl test set covers custom schemes and blocked schemes, but it doesn’t cover the new edge cases introduced by scheme-based detection (e.g. Windows drive-letter paths like C:\\Users\\... / C:/Users/..., or unsafe schemes with leading whitespace). Adding assertions for these would help prevent regressions from the new regex behavior.
| expect(isExternalUrl('//example.com/foo')).toBe(false); | |
| expect(isExternalUrl('file:///Users/test/docs/index.md')).toBe(false); | |
| expect(isExternalUrl('javascript:alert(1)')).toBe(false); | |
| expect(isExternalUrl('data:text/plain,hello')).toBe(false); | |
| expect(isExternalUrl('//example.com/foo')).toBe(false); | |
| expect(isExternalUrl('C:\\Users\\test\\docs\\index.md')).toBe(false); | |
| expect(isExternalUrl('C:/Users/test/docs/index.md')).toBe(false); | |
| expect(isExternalUrl('file:///Users/test/docs/index.md')).toBe(false); | |
| expect(isExternalUrl('javascript:alert(1)')).toBe(false); | |
| expect(isExternalUrl(' javascript:alert(1)')).toBe(false); | |
| expect(isExternalUrl('data:text/plain,hello')).toBe(false); | |
| expect(isExternalUrl(' data:text/plain,hello')).toBe(false); |
editLinkand the themeLinkcomponent regressed for custom URI schemes such asvscode://andjetbrains://, causing them to be treated as relative paths and resolved against the current page URL. This updates external URL detection so valid custom schemes continue to work while preserving existing behavior for normal site links.What changed
http/https/mailto/telchecks inisExternalUrl()with scheme-based detection.javascript:data:file:Where the fix applies
editLinknow preserves custom IDE links such asvscode://file/...Linknow treats custom URI schemes as external instead of resolving them viawindow.locationisExternalUrl()now get the same scheme-aware behaviorRegression coverage
vscode://file/...jetbrains://...https:,mailto:,tel:javascript:,data:, andfile:Example
close [Feature]: support custom URI schemes (vscode://, jetbrains://) in editLink and Link component #3301
Screenshot