fix(network): honor proxy env for node requests (#512)

* fix(network): honor proxy env for node requests

* fix(network): honor default ports in NO_PROXY

* refactor(network): normalize proxy config handling

* refactor(network): delegate proxy env handling to undici

---------

Co-authored-by: jackwener <jakevingoo@gmail.com>

Author: WSL0809

package-lock.json

@@ -58,6 +58,7 @@
     "commander": "^14.0.3",
     "js-yaml": "^4.1.0",
     "turndown": "^7.2.2",
+    "undici": "^7.24.6",
     "ws": "^8.18.0"
   },
   "devDependencies": {

package.json

@@ -2,13 +2,14 @@ import * as fs from 'node:fs';
 import * as http from 'node:http';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { afterEach, describe, expect, it } from 'vitest';
+import { afterEach, describe, expect, it, vi } from 'vitest';
 import { formatCookieHeader, httpDownload, resolveRedirectUrl } from './index.js';
 
 const servers: http.Server[] = [];
 const tempDirs: string[] = [];
 
 afterEach(async () => {
+  vi.unstubAllEnvs();
   await Promise.all(servers.map((server) => new Promise<void>((resolve, reject) => {
     server.close((err) => (err ? reject(err) : resolve()));
   })));
@@ -114,4 +115,21 @@ describe('download helpers', { retry: process.platform === 'win32' ? 2 : 0 }, ()
     expect(forwardedCookie).toBeUndefined();
     expect(fs.readFileSync(destPath, 'utf8')).toBe('ok');
   });
+
+  it('bypasses proxy settings for loopback downloads', async () => {
+    vi.stubEnv('HTTP_PROXY', 'http://127.0.0.1:9');
+
+    const baseUrl = await startServer((_req, res) => {
+      res.statusCode = 200;
+      res.end('ok');
+    });
+
+    const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'opencli-dl-'));
+    tempDirs.push(tempDir);
+    const destPath = path.join(tempDir, 'loopback.txt');
+    const result = await httpDownload(`${baseUrl}/ok`, destPath);
+
+    expect(result).toEqual({ success: true, size: 2 });
+    expect(fs.readFileSync(destPath, 'utf8')).toBe('ok');
+  });
 });

src/download/index.test.ts

@@ -5,16 +5,16 @@
 import { spawn } from 'node:child_process';
 import * as fs from 'node:fs';
 import * as path from 'node:path';
-import * as https from 'node:https';
-import * as http from 'node:http';
 import * as os from 'node:os';
-import { Transform } from 'node:stream';
+import { Readable, Transform } from 'node:stream';
+import type { ReadableStream as WebReadableStream } from 'node:stream/web';
 import { pipeline } from 'node:stream/promises';
 import { URL } from 'node:url';
 import type { ProgressBar } from './progress.js';
 import { isBinaryInstalled } from '../external.js';
 import type { BrowserCookie } from '../types.js';
 import { getErrorMessage } from '../errors.js';
+import { fetchWithNodeNetwork } from '../node-network.js';
 
 export type { BrowserCookie } from '../types.js';
 
@@ -89,9 +89,6 @@ export async function httpDownload(
   const { cookies, headers = {}, timeout = 30000, onProgress, maxRedirects = 10 } = options;
 
   return new Promise((resolve) => {
-    const parsedUrl = new URL(url);
-    const protocol = parsedUrl.protocol === 'https:' ? https : http;
-
     const requestHeaders: Record<string, string> = {
       'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36',
       ...headers,
@@ -118,37 +115,52 @@ export async function httpDownload(
       }
     };
 
-    const request = protocol.get(url, { headers: requestHeaders, timeout }, (response) => {
-      void (async () => {
+    void (async () => {
+      const controller = new AbortController();
+      const timer = setTimeout(() => controller.abort(), timeout);
+      try {
+        const response = await fetchWithNodeNetwork(url, {
+          headers: requestHeaders,
+          signal: controller.signal,
+          redirect: 'manual',
+        });
+        clearTimeout(timer);
+
         // Handle redirects before creating any file handles.
-        if (response.statusCode && response.statusCode >= 300 && response.statusCode < 400 && response.headers.location) {
-          response.resume();
-          if (redirectCount >= maxRedirects) {
-            finish({ success: false, size: 0, error: `Too many redirects (> ${maxRedirects})` });
+        if (response.status >= 300 && response.status < 400) {
+          const location = response.headers.get('location');
+          if (location) {
+            if (redirectCount >= maxRedirects) {
+              finish({ success: false, size: 0, error: `Too many redirects (> ${maxRedirects})` });
+              return;
+            }
+            const redirectUrl = resolveRedirectUrl(url, location);
+            const originalHost = new URL(url).hostname;
+            const redirectHost = new URL(redirectUrl).hostname;
+            const redirectOptions = originalHost === redirectHost
+              ? options
+              : { ...options, cookies: undefined, headers: stripCookieHeaders(options.headers) };
+            finish(await httpDownload(
+              redirectUrl,
+              destPath,
+              redirectOptions,
+              redirectCount + 1,
+            ));
             return;
           }
-          const redirectUrl = resolveRedirectUrl(url, response.headers.location);
-          const originalHost = new URL(url).hostname;
-          const redirectHost = new URL(redirectUrl).hostname;
-          const redirectOptions = originalHost === redirectHost
-            ? options
-            : { ...options, cookies: undefined, headers: stripCookieHeaders(options.headers) };
-          finish(await httpDownload(
-            redirectUrl,
-            destPath,
-            redirectOptions,
-            redirectCount + 1,
-          ));
+        }
+
+        if (response.status !== 200) {
+          finish({ success: false, size: 0, error: `HTTP ${response.status}` });
           return;
         }
 
-        if (response.statusCode !== 200) {
-          response.resume();
-          finish({ success: false, size: 0, error: `HTTP ${response.statusCode}` });
+        if (!response.body) {
+          finish({ success: false, size: 0, error: 'Empty response body' });
           return;
         }
 
-        const totalSize = parseInt(response.headers['content-length'] || '0', 10);
+        const totalSize = parseInt(response.headers.get('content-length') || '0', 10);
         let received = 0;
         const progressStream = new Transform({
           transform(chunk, _encoding, callback) {
@@ -160,26 +172,23 @@ export async function httpDownload(
 
         try {
           await fs.promises.mkdir(path.dirname(destPath), { recursive: true });
-          await pipeline(response, progressStream, fs.createWriteStream(tempPath));
+          await pipeline(
+            Readable.fromWeb(response.body as unknown as WebReadableStream),
+            progressStream,
+            fs.createWriteStream(tempPath),
+          );
           await fs.promises.rename(tempPath, destPath);
           finish({ success: true, size: received });
         } catch (err) {
           await cleanupTempFile();
           finish({ success: false, size: 0, error: getErrorMessage(err) });
         }
-      })();
-    });
-
-    request.on('error', (err) => {
-      void (async () => {
+      } catch (err) {
+        clearTimeout(timer);
         await cleanupTempFile();
-        finish({ success: false, size: 0, error: err.message });
-      })();
-    });
-
-    request.on('timeout', () => {
-      request.destroy(new Error('Timeout'));
-    });
+        finish({ success: false, size: 0, error: err instanceof Error ? err.message : String(err) });
+      }
+    })();
   });
 }
 

src/download/index.ts

@@ -20,8 +20,11 @@ import { discoverClis, discoverPlugins } from './discovery.js';
 import { getCompletions } from './completion.js';
 import { runCli } from './cli.js';
 import { emitHook } from './hooks.js';
+import { installNodeNetwork } from './node-network.js';
 import { registerUpdateNoticeOnExit, checkForUpdateBackground } from './update-check.js';
 
+installNodeNetwork();
+
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
 const BUILTIN_CLIS = path.resolve(__dirname, 'clis');

src/main.ts

@@ -0,0 +1,93 @@
+import { describe, expect, it } from 'vitest';
+
+import { decideProxy, hasProxyEnv } from './node-network.js';
+
+describe('node network proxy decisions', () => {
+  it('detects common proxy env variables', () => {
+    expect(hasProxyEnv({ https_proxy: 'http://127.0.0.1:7897' })).toBe(true);
+    expect(hasProxyEnv({ HTTP_PROXY: 'http://proxy.example:8080' })).toBe(true);
+    expect(hasProxyEnv({})).toBe(false);
+  });
+
+  it('routes external https traffic through https_proxy', () => {
+    const decision = decideProxy(
+      new URL('https://www.v2ex.com/api/topics/latest.json'),
+      { https_proxy: 'http://127.0.0.1:7897' },
+    );
+
+    expect(decision).toEqual({
+      mode: 'proxy',
+      proxyUrl: 'http://127.0.0.1:7897',
+    });
+  });
+
+  it('falls back to HTTP_PROXY for https traffic when HTTPS_PROXY is absent', () => {
+    const decision = decideProxy(
+      new URL('https://www.v2ex.com/api/topics/latest.json'),
+      { HTTP_PROXY: 'http://127.0.0.1:7897' },
+    );
+
+    expect(decision).toEqual({
+      mode: 'proxy',
+      proxyUrl: 'http://127.0.0.1:7897',
+    });
+  });
+
+  it('bypasses proxies for loopback addresses', () => {
+    const env = { https_proxy: 'http://127.0.0.1:7897', http_proxy: 'http://127.0.0.1:7897' };
+
+    expect(decideProxy(new URL('http://127.0.0.1:19825/status'), env)).toEqual({ mode: 'direct' });
+    expect(decideProxy(new URL('http://localhost:19825/status'), env)).toEqual({ mode: 'direct' });
+    expect(decideProxy(new URL('http://[::1]:19825/status'), env)).toEqual({ mode: 'direct' });
+  });
+
+  it('honors NO_PROXY domain matches', () => {
+    const decision = decideProxy(
+      new URL('https://api.example.com/v1/items'),
+      {
+        https_proxy: 'http://127.0.0.1:7897',
+        no_proxy: '.example.com',
+      },
+    );
+
+    expect(decision).toEqual({ mode: 'direct' });
+  });
+
+  it('supports wildcard-style NO_PROXY subdomain entries', () => {
+    const decision = decideProxy(
+      new URL('https://api.example.com/v1/items'),
+      {
+        https_proxy: 'http://127.0.0.1:7897',
+        no_proxy: '*.example.com',
+      },
+    );
+
+    expect(decision).toEqual({ mode: 'direct' });
+  });
+
+  it('matches NO_PROXY entries that rely on the default URL port', () => {
+    const env = { https_proxy: 'http://127.0.0.1:7897', http_proxy: 'http://127.0.0.1:7897' };
+
+    expect(decideProxy(
+      new URL('https://example.com/'),
+      { ...env, NO_PROXY: 'example.com:443' },
+    )).toEqual({ mode: 'direct' });
+
+    expect(decideProxy(
+      new URL('http://example.com/health'),
+      { ...env, NO_PROXY: 'example.com:80' },
+    )).toEqual({ mode: 'direct' });
+  });
+
+  it('falls back to ALL_PROXY when protocol-specific settings are absent', () => {
+    const decision = decideProxy(
+      new URL('http://example.net/data'),
+      { ALL_PROXY: 'socks5://127.0.0.1:1080' },
+    );
+
+    expect(decision).toEqual({
+      mode: 'proxy',
+      proxyUrl: 'socks5://127.0.0.1:1080',
+    });
+  });
+});