fix: resolve relative URLs in client-side fetch/XHR patches#5
Conversation
the fetch/XHR patches only caught absolute URLs (http:// / https://). relative URLs like /api/data or assets/chunk.js resolved against the proxy origin (localhost:8788) instead of the target site, causing API calls and dynamic script loading to 404. adds resolveForProxy() which extracts the target origin from window.location.pathname and resolves relative URLs against it before proxying. handles all edge cases: /browse/ paths, data/blob URIs, absolute URLs, and root-relative/relative paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the client-side fetch/XHR interception used on proxied pages by ensuring relative URLs are resolved against the target site (not the proxy origin) before being re-proxied, reducing 404s for API calls and dynamically loaded resources.
Changes:
- Add a
resolveForProxy()helper in the injected client script to convert relative/absolute URLs into proxy-routed URLs. - Update the fetch and XHR patches to route all string URLs through
resolveForProxy(). - Add an integration test asserting the new helper is injected into rewritten HTML.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/uppercase-script.ts |
Adds resolveForProxy() and uses it in fetch/XHR patching to handle relative URLs. |
src/rewriter.test.ts |
Adds a test asserting the new helper is injected into proxied HTML. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function resolveForProxy(url) { | ||
| if (!url || url.startsWith(PROXY_PREFIX) || url.startsWith('data:') || url.startsWith('blob:') || url.startsWith('javascript:') || url.startsWith('#')) { | ||
| return url; | ||
| } |
There was a problem hiding this comment.
resolveForProxy() claims to handle already-proxied /browse/ paths, but it only checks url.startsWith(PROXY_PREFIX). If application code constructs an absolute proxied URL (e.g. new URL('/api', location.href).toString() on a proxied page -> https://<proxy-origin>/browse/https://<target>/api), this function treats it as an absolute https:// URL and prefixes again, producing a broken double-proxy URL. Consider detecting proxy-origin absolute URLs whose pathname starts with PROXY_PREFIX and returning them unchanged (or normalized to a relative /browse/...), before the http(s) branch.
| } | |
| } | |
| // handle absolute URLs that already point to this origin with a /browse/ path | |
| try { | |
| var absolute = new URL(url, window.location.href); | |
| if (absolute.origin === window.location.origin && absolute.pathname.startsWith(PROXY_PREFIX)) { | |
| return url; | |
| } | |
| } catch(e) {} |
| } else if (input instanceof Request && !input.url.includes(PROXY_PREFIX)) { | ||
| input = new Request(resolveForProxy(input.url), input); |
There was a problem hiding this comment.
The Request branch uses !input.url.includes(PROXY_PREFIX) to decide whether to rewrite. This can incorrectly skip proxying for legitimate target URLs that merely contain /browse/ in their path (e.g. https://example.com/browse/assets.js). Prefer checking whether the URL is already proxied by parsing it and verifying the proxy-origin + pathname.startsWith(PROXY_PREFIX) instead of substring matching.
| function resolveForProxy(url) { | ||
| if (!url || url.startsWith(PROXY_PREFIX) || url.startsWith('data:') || url.startsWith('blob:') || url.startsWith('javascript:') || url.startsWith('#')) { | ||
| return url; | ||
| } | ||
| if (url.startsWith('http://') || url.startsWith('https://')) { | ||
| return PROXY_PREFIX + url; | ||
| } | ||
| // relative URL — resolve against the target origin | ||
| var path = window.location.pathname; | ||
| if (path.startsWith(PROXY_PREFIX)) { | ||
| var targetUrl = path.slice(PROXY_PREFIX.length) + window.location.search; | ||
| try { | ||
| var resolved = new URL(url, targetUrl).href; | ||
| return PROXY_PREFIX + resolved; | ||
| } catch(e) {} | ||
| } | ||
| return url; |
There was a problem hiding this comment.
resolveForProxy() only special-cases a few non-network schemes (data:, blob:, javascript:). For fetch/XHR, other schemes like mailto:, tel:, chrome-extension: etc. should also be left untouched; currently they can be treated as "relative" and end up rewritten into /browse/<scheme>:..., which is guaranteed to fail. Consider a generic scheme check (e.g. detect ^[a-zA-Z][a-zA-Z0-9+.-]*: and only proxy http:/https:) to avoid rewriting unsupported schemes.
Summary
http:///https://). Relative URLs like/api/dataorassets/chunk.jsresolved against the proxy origin instead of the target site, causing 404s for API calls and dynamic script loading.resolveForProxy()helper that extracts the target origin fromwindow.location.pathnameand resolves relative URLs against it before proxying/browse/paths,data:/blob:URIs, absolute URLs, root-relative paths, and relative pathsTest plan
resolveForProxyinjection test)🤖 Generated with Claude Code