fix(http): treat http://localhost as base URL for relative paths to a…#4
fix(http): treat http://localhost as base URL for relative paths to a…#4MitchLewis930 wants to merge 1 commit intopr_024_beforefrom
Conversation
…void `ERR_INVALID_URL` error; (axios#5528)
📝 WalkthroughWalkthroughThe HTTP adapter now constructs URLs from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/adapters/http.js`:
- Around line 184-187: The change that constructs parsed = new URL(fullPath,
'http://localhost') causes relative URLs to silently resolve to localhost;
update the HTTP adapter to validate input before creating the URL: in the same
area where buildFullPath(config.baseURL, config.url) and parsed are used, detect
if config.url (or the resulting fullPath) is a relative or scheme-relative URL
and config.baseURL is not provided, and then either throw a clear error
rejecting relative URLs (SSRF guard) or require callers to set baseURL; add unit
tests covering calls like axios.get('/api/users') without baseURL to assert the
new behavior and include a test for scheme-relative URLs (//host) as well, and
reference supportedProtocols, parsed, and protocol handling so existing fallback
logic remains consistent.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/adapters/http.js
🧰 Additional context used
🧬 Code graph analysis (1)
lib/adapters/http.js (2)
lib/helpers/isURLSameOrigin.js (1)
parsed(56-56)lib/adapters/xhr.js (1)
fullPath(77-77)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Parse url | ||
| const fullPath = buildFullPath(config.baseURL, config.url); | ||
| const parsed = new URL(fullPath); | ||
| const parsed = new URL(fullPath, 'http://localhost'); | ||
| const protocol = parsed.protocol || supportedProtocols[0]; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js WHATWG URL behavior relative URLs scheme-relative new URL base
💡 Result:
In Node.js, the WHATWG URL implementation follows the WHATWG URL Standard: new URL(input, base) parses input as either absolute (base ignored) or relative (base required). If parsing fails, it throws TypeError. [1][2]
Key behaviors (relative vs absolute)
- Absolute URL input (has a scheme like
https:):baseis ignored. [1]new URL('https://b.example/x', 'https://a.example/y').href // "https://b.example/x"
- Relative URL input (no scheme):
baseis required. [1][2]new URL('/x', 'https://a.example/dir/page').href // "https://a.example/x" new URL('x', 'https://a.example/dir/page').href // "https://a.example/dir/x"
Scheme-relative (a.k.a. protocol-relative) URLs: "//host/path"
In the WHATWG standard these are scheme-relative URLs and they are relative URL strings, meaning they require a base URL to resolve. [2]
When resolved, they inherit the scheme from the base and replace the authority (host/port/etc.); the base’s path is not used except for the scheme selection. [2]
new URL('//cdn.example/lib.js', 'https://site.example/a/b').href
// "https://cdn.example/lib.js"
new URL('//cdn.example/lib.js', 'http://site.example/a/b').href
// "http://cdn.example/lib.js"What fails
- A relative (including scheme-relative) URL without a base throws:
new URL('//cdn.example/lib.js') // TypeError
new URL('/x') // TypeErrorBecause a “non-null base URL is necessary when parsing a relative-URL string.” [2]
Sources: Node.js URL docs for new URL(input[, base]) semantics [1]; WHATWG URL Standard definitions of relative-URL strings and scheme-relative URLs, and the requirement for a base [2].
🏁 Script executed:
# Find and examine the file context around line 186
fd -t f "http.js" lib/adapters/Repository: Signal65/axios-CodeRabbit
Length of output: 89
🏁 Script executed:
# Search for how buildFullPath works and what it returns
rg -A 5 -B 5 "buildFullPath" lib/adapters/http.js | head -50Repository: Signal65/axios-CodeRabbit
Length of output: 755
🏁 Script executed:
# Check for tests related to URL handling
fd -t f "test\|spec" | xargs rg "baseURL\|buildFullPath\|localhost" -l 2>/dev/null | head -10Repository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
# Examine buildFullPath implementation
cat -n core/buildFullPath.jsRepository: Signal65/axios-CodeRabbit
Length of output: 122
🏁 Script executed:
# Search for tests related to URL handling or baseURL
find . -type f \( -name "*.test.js" -o -name "*.spec.js" -o -path "*/test/*" \) | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 709
🏁 Script executed:
# Check if there are any guards or validation before the URL parsing
rg -B 10 -A 10 "new URL\(fullPath" lib/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 735
🏁 Script executed:
# Look for relative URL tests or baseURL tests
rg "baseURL.*relative|relative.*URL|\\/path|scheme-relative" -t js | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 221
🏁 Script executed:
# Find buildFullPath.js
fd -t f "buildFullPath" . --extension jsRepository: Signal65/axios-CodeRabbit
Length of output: 136
🏁 Script executed:
# Look for unit/spec tests
fd -t f -path "*/test/*" "http\|url\|adapter" --extension js | head -30Repository: Signal65/axios-CodeRabbit
Length of output: 237
🏁 Script executed:
# Search for buildFullPath usage and definition
rg "buildFullPath" -A 5 -B 2 | head -80Repository: Signal65/axios-CodeRabbit
Length of output: 4451
🏁 Script executed:
# Find http adapter test file
fd -t f "http" test/specs --extension jsRepository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
# View the buildFullPath implementation completely
cat -n lib/core/buildFullPath.jsRepository: Signal65/axios-CodeRabbit
Length of output: 910
🏁 Script executed:
# Check for tests in http adapter specs for relative URLs
fd -t f "adapter" test/specs --extension js | xargs rg -l "http" 2>/dev/nullRepository: Signal65/axios-CodeRabbit
Length of output: 53
🏁 Script executed:
# Look at http adapter tests
find test -name "*http*" -type fRepository: Signal65/axios-CodeRabbit
Length of output: 95
🏁 Script executed:
# Examine the http adapter test file for relative URL tests
cat -n test/unit/adapters/http.js | head -150Repository: Signal65/axios-CodeRabbit
Length of output: 5350
🏁 Script executed:
# Search for relative URL, baseURL, or localhost in the test file
rg -n "relative|baseURL|localhost|undefined.*config.url" test/unit/adapters/http.js | head -30Repository: Signal65/axios-CodeRabbit
Length of output: 1859
🏁 Script executed:
# Check file size first
wc -l test/unit/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 100
🏁 Script executed:
# Search for tests with relative URLs (starting with / but no baseURL)
rg -n "url.*['\"]/" test/unit/adapters/http.js | grep -v "http\|https" | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 119
🏁 Script executed:
# Search for baseURL configurations in tests
rg -n "baseURL" test/unit/adapters/http.js | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 251
🏁 Script executed:
# Look for tests that might pass only url without baseURL
rg -B 5 -A 5 "config.*url" test/unit/adapters/http.js | head -60Repository: Signal65/axios-CodeRabbit
Length of output: 369
🏁 Script executed:
# Check git history for this specific change
git log -p --all -S "new URL(fullPath, 'http://localhost')" -- lib/adapters/http.js 2>/dev/null | head -100Repository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
# Confirm there are no tests for relative URLs without baseURL
rg -n "axios\.(get|post|request)" test/unit/adapters/http.js | grep -v "baseURL\|http://" | head -10Repository: Signal65/axios-CodeRabbit
Length of output: 675
🏁 Script executed:
# Check the exact lines around the URL parsing change to see context
sed -n '175,195p' lib/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 747
Relative URLs now silently resolve to localhost instead of throwing—confirm this is intentional and add test coverage or validation guards.
At line 186, the change to new URL(fullPath, 'http://localhost') enables relative URLs (e.g., /path) and scheme-relative URLs (e.g., //host) to resolve against localhost rather than throw. This is a behavioral change: axios.get('/api/users') without baseURL now makes a real request to http://localhost/api/users instead of failing.
If callers pass untrusted or partially-validated relative URLs without baseURL, they will silently hit localhost or internal services, creating an SSRF risk. Existing tests cover only relative URLs with baseURL set; the new behavior is untested.
Either reject relative URLs unless baseURL is configured, document this as intentional with migration guidance, or add comprehensive test coverage for the localhost fallback behavior.
🤖 Prompt for AI Agents
In `@lib/adapters/http.js` around lines 184 - 187, The change that constructs
parsed = new URL(fullPath, 'http://localhost') causes relative URLs to silently
resolve to localhost; update the HTTP adapter to validate input before creating
the URL: in the same area where buildFullPath(config.baseURL, config.url) and
parsed are used, detect if config.url (or the resulting fullPath) is a relative
or scheme-relative URL and config.baseURL is not provided, and then either throw
a clear error rejecting relative URLs (SSRF guard) or require callers to set
baseURL; add unit tests covering calls like axios.get('/api/users') without
baseURL to assert the new behavior and include a test for scheme-relative URLs
(//host) as well, and reference supportedProtocols, parsed, and protocol
handling so existing fallback logic remains consistent.
2 participants
PR_024
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.