Skip to content

fix: resolve localhost requests with IP literal URLs#289

Merged
nsrCodes merged 1 commit intomasterfrom
fix/localhost-ip-literal-resolution
Feb 16, 2026
Merged

fix: resolve localhost requests with IP literal URLs#289
nsrCodes merged 1 commit intomasterfrom
fix/localhost-ip-literal-resolution

Conversation

@nsrCodes
Copy link
Collaborator

@nsrCodes nsrCodes commented Feb 13, 2026

Summary

Follow-up to #276. The localhost DNS lookup fix from that PR doesn't work when the request URL contains a raw IP address (127.0.0.1, [::1], 0.0.0.0) instead of the localhost hostname. Two issues:

  1. Custom DNS lookup is skipped for IP literals. Node.js only calls the agent's lookup function for hostnames — when the URL already contains an IP, it connects directly. So http://127.0.0.1:3000 bypasses the custom lookup entirely and always connects via IPv4, even if the server only listens on IPv6.

  2. IPv6 hostname check never matched. new URL("http://[::1]:3000").hostname returns [::1] (with brackets), but the code compared against ::1 (without brackets).

This PR fixes both by:

  • Comparing against the bracketed [::1] form and also matching 0.0.0.0
  • Rewriting IP literal URLs to the concrete working IP address (determined via checkConnection) when the current hostname won't connect

Test plan

  • Request http://127.0.0.1:<port> against an IPv6-only local server — should succeed
  • Request http://[::1]:<port> against an IPv4-only local server — should succeed
  • Request http://0.0.0.0:<port> against both IPv4-only and IPv6-only servers — should succeed
  • Request http://localhost:<port> — should continue working as before
  • Non-local URLs (e.g. http://example.com) — should not be intercepted

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Refactoring
    • Enhanced detection and routing of proxied network requests to ensure proper connectivity across different network configurations.

Follow-up to #276. The custom DNS lookup added in that PR is never
invoked by Node.js when the URL contains a raw IP literal (127.0.0.1,
[::1], 0.0.0.0) because Node skips DNS resolution for IPs entirely.
Additionally, `new URL("http://[::1]:3000").hostname` returns `[::1]`
(with brackets), so the original IPv6 hostname check never matched.

Fix both issues by:
1. Comparing against the bracketed `[::1]` form and adding `0.0.0.0`
2. Rewriting IP literal URLs to the concrete working address when the
   hostname isn't already correct

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Walkthrough

The change modifies src/main/actions/getProxiedAxios.ts to enhance localhost detection and request routing. It introduces a new constant for unspecified addresses (0.0.0.0) and extends localhost detection to recognize IPv4, IPv6 ([::1]), and unspecified address formats. When localhost-like hostnames are detected, the code installs a custom DNS lookup for the target port. For non-localhost hostnames with localhost-like addresses, it rewrites the request URL to a working IP address (preferring IPv6 when reachable, otherwise IPv4) since Node.js skips DNS resolution for raw IP literals. HTTP/HTTPS agent creation with the new lookup logic is retained while adding conditional URL rewriting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • wrongsahil
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: resolve localhost requests with IP literal URLs' accurately describes the main change: handling IP literal URLs (127.0.0.1, [::1], 0.0.0.0) by rewriting them to working IP addresses for proper routing.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/localhost-ip-literal-resolution

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/main/actions/getProxiedAxios.ts (2)

116-128: Duplicate checkConnection probe — redundant I/O and possible inconsistency.

createLocalhostLookup(port) on line 116 already probes LOCAL_IPV6 on the target port, then line 123 probes the same endpoint again. This doubles the connection attempts (up to 2 × 1 s timeout on failure) and could theoretically yield different results between the two calls.

Consider refactoring so the IPv6 reachability result is computed once and shared with both the lookup creation and the URL rewriting logic.

♻️ Proposed refactor to eliminate the duplicate probe
       if (isLocalhost) {
         const port = urlPort ? parseInt(urlPort, 10) : protocol === "https:" ? 443 : 80;
 
-        const lookup = await createLocalhostLookup(port);
+        const ipv6Works = await checkConnection(LOCAL_IPV6, port).catch(() => false);
+        const targetIp = ipv6Works ? LOCAL_IPV6 : LOCAL_IPV4;
+        const targetFamily = ipv6Works ? 6 : 4;
+        const lookup = (_hostname: string, _options: any, callback: any) => {
+          callback(null, targetIp, targetFamily);
+        };
         requestConfig.httpAgent = new http.Agent({ lookup });
         requestConfig.httpsAgent = new https.Agent({ lookup });
 
         // Node.js skips DNS lookup for raw IP literals, so the custom lookup
         // above has no effect. Rewrite the URL to the concrete working IP.
         if (hostname !== "localhost") {
-          const ipv6Works = await checkConnection(LOCAL_IPV6, port).catch(() => false);
           const targetIp = ipv6Works ? `[${LOCAL_IPV6}]` : LOCAL_IPV4;
 
           if (hostname !== targetIp) {
             requestConfig.url = requestUrl.replace(hostname, targetIp);
           }
         }
       }

Note: the inner targetIp for URL rewriting needs the bracketed form, so you'd use a separate variable (e.g., const targetUrlHost = ipv6Works ? \[${LOCAL_IPV6}]` : LOCAL_IPV4;`).


127-127: Fragile string-based URL rewriting — consider using the URL object.

requestUrl.replace(hostname, targetIp) relies on the first occurrence of hostname being the host portion. While unlikely to cause issues with these specific IP literals, it's brittle if the hostname string ever appears in the path, query, or credentials. Since you already have a parsed URL object (url), mutating url.hostname and using url.toString() would be safer.

♻️ Safer URL rewriting
-          if (hostname !== targetIp) {
-            requestConfig.url = requestUrl.replace(hostname, targetIp);
-          }
+          if (hostname !== targetIp) {
+            url.hostname = targetIp;
+            requestConfig.url = url.toString();
+          }

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nsrCodes nsrCodes merged commit c00d140 into master Feb 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments