From 6e6f1a24900017b6a4e96ce5179d95136d9ad9f5 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 3 Jul 2025 20:11:13 +0200 Subject: [PATCH 1/2] test: move http proxy tests to test/client-proxy Rewrite to ESM to use TLA. Also add a test to make sure case precedence is honored. Refs: https://about.gitlab.com/blog/we-need-to-talk-no-proxy --- test/client-proxy/client-proxy.status | 7 ++ test/client-proxy/test-http-proxy-fetch.mjs | 76 ++++++++++++++++++ test/client-proxy/test-https-proxy-fetch.mjs | 83 ++++++++++++++++++++ test/client-proxy/testcfg.py | 6 ++ test/common/proxy-server.js | 78 ++++++++++++++++-- test/parallel/test-http-proxy-fetch.js | 62 --------------- test/parallel/test-https-proxy-fetch.js | 67 ---------------- 7 files changed, 243 insertions(+), 136 deletions(-) create mode 100644 test/client-proxy/client-proxy.status create mode 100644 test/client-proxy/test-http-proxy-fetch.mjs create mode 100644 test/client-proxy/test-https-proxy-fetch.mjs create mode 100644 test/client-proxy/testcfg.py delete mode 100644 test/parallel/test-http-proxy-fetch.js delete mode 100644 test/parallel/test-https-proxy-fetch.js diff --git a/test/client-proxy/client-proxy.status b/test/client-proxy/client-proxy.status new file mode 100644 index 00000000000000..33cdae9612b2af --- /dev/null +++ b/test/client-proxy/client-proxy.status @@ -0,0 +1,7 @@ +prefix client-proxy + +# To mark a test as flaky, list the test name in the appropriate section +# below, without ".js", followed by ": PASS,FLAKY". Example: +# sample-test : PASS,FLAKY + +[true] # This section applies to all platforms diff --git a/test/client-proxy/test-http-proxy-fetch.mjs b/test/client-proxy/test-http-proxy-fetch.mjs new file mode 100644 index 00000000000000..bfbd4e911816fb --- /dev/null +++ b/test/client-proxy/test-http-proxy-fetch.mjs @@ -0,0 +1,76 @@ +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import { once } from 'events'; +import http from 'node:http'; +import { createProxyServer, checkProxiedFetch } from '../common/proxy-server.js'; + +// Start a server to process the final request. +const server = http.createServer(common.mustCall((req, res) => { + res.end('Hello world'); +}, 3)); +server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); +server.listen(0); +await once(server, 'listening'); + +// Start a minimal proxy server. +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +const serverHost = `localhost:${server.address().port}`; + +// FIXME(undici:4083): undici currently always tunnels the request over +// CONNECT if proxyTunnel is not explicitly set to false, but what we +// need is for it to be automatically false for HTTP requests to be +// consistent with curl. +const expectedLogs = [{ + method: 'CONNECT', + url: serverHost, + headers: { + 'connection': 'close', + 'host': serverHost, + 'proxy-connection': 'keep-alive', + }, +}]; + +// Check upper-cased HTTPS_PROXY environment variable. +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `http://${serverHost}/test`, + HTTP_PROXY: `http://localhost:${proxy.address().port}`, +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +// Check lower-cased https_proxy environment variable. +logs.splice(0, logs.length); +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `http://${serverHost}/test`, + http_proxy: `http://localhost:${proxy.address().port}`, +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +const proxy2 = http.createServer(); +proxy2.on('connect', common.mustNotCall()); +proxy2.listen(0); +await once(proxy2, 'listening'); + +// Check lower-cased http_proxy environment variable takes precedence. +logs.splice(0, logs.length); +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `http://${serverHost}/test`, + http_proxy: `http://localhost:${proxy.address().port}`, + HTTP_PROXY: `http://localhost:${proxy2.address().port}`, +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +proxy.close(); +proxy2.close(); +server.close(); diff --git a/test/client-proxy/test-https-proxy-fetch.mjs b/test/client-proxy/test-https-proxy-fetch.mjs new file mode 100644 index 00000000000000..8a3d7f07284e9a --- /dev/null +++ b/test/client-proxy/test-https-proxy-fetch.mjs @@ -0,0 +1,83 @@ +import * as common from '../common/index.mjs'; +import fixtures from '../common/fixtures.js'; +import assert from 'node:assert'; +import https from 'node:https'; +import http from 'node:http'; +import { once } from 'events'; +import { createProxyServer, checkProxiedFetch } from '../common/proxy-server.js'; + +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Start a server to process the final request. +const server = https.createServer({ + cert: fixtures.readKey('agent8-cert.pem'), + key: fixtures.readKey('agent8-key.pem'), +}, common.mustCall((req, res) => { + res.end('Hello world'); +}, 3)); +server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); +server.listen(0); +await once(server, 'listening'); + +// Start a minimal proxy server. +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +const serverHost = `localhost:${server.address().port}`; + +const expectedLogs = [{ + method: 'CONNECT', + url: serverHost, + headers: { + 'connection': 'close', + 'host': serverHost, + 'proxy-connection': 'keep-alive', + }, +}]; + +// Check upper-cased HTTPS_PROXY environment variable. +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `https://${serverHost}/test`, + HTTPS_PROXY: `http://localhost:${proxy.address().port}`, + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +// Check lower-cased https_proxy environment variable. +logs.splice(0, logs.length); +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `https://${serverHost}/test`, + https_proxy: `http://localhost:${proxy.address().port}`, + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +const proxy2 = http.createServer(); +proxy2.on('connect', common.mustNotCall()); +proxy2.listen(0); +await once(proxy2, 'listening'); + +// Check lower-cased https_proxy environment variable takes precedence. +logs.splice(0, logs.length); +await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `https://${serverHost}/test`, + https_proxy: `http://localhost:${proxy.address().port}`, + HTTPS_PROXY: `http://localhost:${proxy2.address().port}`, + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), +}, { + stdout: 'Hello world', +}); +assert.deepStrictEqual(logs, expectedLogs); + +proxy.close(); +proxy2.close(); +server.close(); diff --git a/test/client-proxy/testcfg.py b/test/client-proxy/testcfg.py new file mode 100644 index 00000000000000..055549e7a04c5e --- /dev/null +++ b/test/client-proxy/testcfg.py @@ -0,0 +1,6 @@ +import sys, os +sys.path.append(os.path.join(os.path.dirname(__file__), '..')) +import testpy + +def GetConfiguration(context, root): + return testpy.ParallelTestConfiguration(context, root, 'client-proxy') diff --git a/test/common/proxy-server.js b/test/common/proxy-server.js index d2da6813b5a971..5777d7e2519f3e 100644 --- a/test/common/proxy-server.js +++ b/test/common/proxy-server.js @@ -14,19 +14,32 @@ function logRequest(logs, req) { // This creates a minimal proxy server that logs the requests it gets // to an array before performing proxying. -exports.createProxyServer = function() { +exports.createProxyServer = function(options = {}) { const logs = []; - const proxy = http.createServer(); + let proxy; + if (options.https) { + const common = require('../common'); + if (!common.hasCrypto) { + common.skip('missing crypto'); + } + proxy = require('https').createServer({ + cert: require('./fixtures').readKey('agent9-cert.pem'), + key: require('./fixtures').readKey('agent9-key.pem'), + }); + } else { + proxy = http.createServer(); + } proxy.on('request', (req, res) => { logRequest(logs, req); const [hostname, port] = req.headers.host.split(':'); const targetPort = port || 80; + const url = new URL(req.url); const options = { hostname: hostname, port: targetPort, - path: req.url, + path: url.pathname + url.search, // Convert back to relative URL. method: req.method, headers: req.headers, }; @@ -38,8 +51,16 @@ exports.createProxyServer = function() { proxyReq.on('error', (err) => { logs.push({ error: err, source: 'proxy request' }); - res.writeHead(500); - res.end('Proxy error: ' + err.message); + if (!res.headersSent) { + res.writeHead(500); + } + if (!res.writableEnded) { + res.end(`Proxy error ${err.code}: ${err.message}`); + } + }); + + res.on('error', (err) => { + logs.push({ error: err, source: 'proxy response' }); }); req.pipe(proxyReq, { end: true }); @@ -49,6 +70,11 @@ exports.createProxyServer = function() { logRequest(logs, req); const [hostname, port] = req.url.split(':'); + + res.on('error', (err) => { + logs.push({ error: err, source: 'proxy response' }); + }); + const proxyReq = net.connect(port, hostname, () => { res.write( 'HTTP/1.1 200 Connection Established\r\n' + @@ -74,8 +100,46 @@ exports.createProxyServer = function() { return { proxy, logs }; }; -exports.checkProxiedRequest = async function(envExtension, expectation) { - const { spawnPromisified } = require('./'); +function spawnPromisified(...args) { + const { spawn } = require('child_process'); + let stderr = ''; + let stdout = ''; + + const child = spawn(...args); + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (data) => { + console.error('[STDERR]', data); + stderr += data; + }); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (data) => { + console.log('[STDOUT]', data); + stdout += data; + }); + + return new Promise((resolve, reject) => { + child.on('close', (code, signal) => { + console.log('[CLOSE]', code, signal); + resolve({ + code, + signal, + stderr, + stdout, + }); + }); + child.on('error', (code, signal) => { + console.log('[ERROR]', code, signal); + reject({ + code, + signal, + stderr, + stdout, + }); + }); + }); +} + +exports.checkProxiedFetch = async function(envExtension, expectation) { const fixtures = require('./fixtures'); const { code, signal, stdout, stderr } = await spawnPromisified( process.execPath, diff --git a/test/parallel/test-http-proxy-fetch.js b/test/parallel/test-http-proxy-fetch.js deleted file mode 100644 index 7e2f7c2eca5ee9..00000000000000 --- a/test/parallel/test-http-proxy-fetch.js +++ /dev/null @@ -1,62 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const { once } = require('events'); -const http = require('http'); -const { createProxyServer, checkProxiedRequest } = require('../common/proxy-server'); - -(async () => { - // Start a server to process the final request. - const server = http.createServer(common.mustCall((req, res) => { - res.end('Hello world'); - }, 2)); - server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); - server.listen(0); - await once(server, 'listening'); - - // Start a minimal proxy server. - const { proxy, logs } = createProxyServer(); - proxy.listen(0); - await once(proxy, 'listening'); - - const serverHost = `localhost:${server.address().port}`; - - // FIXME(undici:4083): undici currently always tunnels the request over - // CONNECT if proxyTunnel is not explicitly set to false, but what we - // need is for it to be automatically false for HTTP requests to be - // consistent with curl. - const expectedLogs = [{ - method: 'CONNECT', - url: serverHost, - headers: { - 'connection': 'close', - 'host': serverHost, - 'proxy-connection': 'keep-alive' - } - }]; - - // Check upper-cased HTTPS_PROXY environment variable. - await checkProxiedRequest({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `http://${serverHost}/test`, - HTTP_PROXY: `http://localhost:${proxy.address().port}`, - }, { - stdout: 'Hello world', - }); - assert.deepStrictEqual(logs, expectedLogs); - - // Check lower-cased https_proxy environment variable. - logs.splice(0, logs.length); - await checkProxiedRequest({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `http://${serverHost}/test`, - http_proxy: `http://localhost:${proxy.address().port}`, - }, { - stdout: 'Hello world', - }); - assert.deepStrictEqual(logs, expectedLogs); - - proxy.close(); - server.close(); -})().then(common.mustCall()); diff --git a/test/parallel/test-https-proxy-fetch.js b/test/parallel/test-https-proxy-fetch.js deleted file mode 100644 index b4dab4e3eebb6b..00000000000000 --- a/test/parallel/test-https-proxy-fetch.js +++ /dev/null @@ -1,67 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -const fixtures = require('../common/fixtures'); -const assert = require('assert'); -const https = require('https'); -const { once } = require('events'); -const { createProxyServer, checkProxiedRequest } = require('../common/proxy-server'); - -(async () => { - // Start a server to process the final request. - const server = https.createServer({ - cert: fixtures.readKey('agent8-cert.pem'), - key: fixtures.readKey('agent8-key.pem'), - }, common.mustCall((req, res) => { - res.end('Hello world'); - }, 2)); - server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); - server.listen(0); - await once(server, 'listening'); - - // Start a minimal proxy server. - const { proxy, logs } = createProxyServer(); - proxy.listen(0); - await once(proxy, 'listening'); - - const serverHost = `localhost:${server.address().port}`; - - const expectedLogs = [{ - method: 'CONNECT', - url: serverHost, - headers: { - 'connection': 'close', - 'host': serverHost, - 'proxy-connection': 'keep-alive' - } - }]; - - // Check upper-cased HTTPS_PROXY environment variable. - await checkProxiedRequest({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `https://${serverHost}/test`, - HTTPS_PROXY: `http://localhost:${proxy.address().port}`, - NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), - }, { - stdout: 'Hello world', - }); - assert.deepStrictEqual(logs, expectedLogs); - - // Check lower-cased https_proxy environment variable. - logs.splice(0, logs.length); - await checkProxiedRequest({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `https://${serverHost}/test`, - https_proxy: `http://localhost:${proxy.address().port}`, - NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), - }, { - stdout: 'Hello world', - }); - assert.deepStrictEqual(logs, expectedLogs); - - proxy.close(); - server.close(); -})().then(common.mustCall()); From ff6ba64a9d6ef3544dde51d452fec345d479dbf6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 10 Jul 2025 20:08:44 +0200 Subject: [PATCH 2/2] fixup! test: move http proxy tests to test/client-proxy --- test/client-proxy/test-http-proxy-fetch.mjs | 38 ++++++++-------- test/client-proxy/test-https-proxy-fetch.mjs | 46 ++++++++++++-------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/test/client-proxy/test-http-proxy-fetch.mjs b/test/client-proxy/test-http-proxy-fetch.mjs index bfbd4e911816fb..493ff022e1aa74 100644 --- a/test/client-proxy/test-http-proxy-fetch.mjs +++ b/test/client-proxy/test-http-proxy-fetch.mjs @@ -7,7 +7,7 @@ import { createProxyServer, checkProxiedFetch } from '../common/proxy-server.js' // Start a server to process the final request. const server = http.createServer(common.mustCall((req, res) => { res.end('Hello world'); -}, 3)); +}, common.isWindows ? 2 : 3)); server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); server.listen(0); await once(server, 'listening'); @@ -54,23 +54,27 @@ await checkProxiedFetch({ }); assert.deepStrictEqual(logs, expectedLogs); -const proxy2 = http.createServer(); -proxy2.on('connect', common.mustNotCall()); -proxy2.listen(0); -await once(proxy2, 'listening'); - // Check lower-cased http_proxy environment variable takes precedence. -logs.splice(0, logs.length); -await checkProxiedFetch({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `http://${serverHost}/test`, - http_proxy: `http://localhost:${proxy.address().port}`, - HTTP_PROXY: `http://localhost:${proxy2.address().port}`, -}, { - stdout: 'Hello world', -}); -assert.deepStrictEqual(logs, expectedLogs); +// On Windows, environment variables are case-insensitive, so this test +// is not applicable. +if (!common.isWindows) { + const proxy2 = http.createServer(); + proxy2.on('connect', common.mustNotCall()); + proxy2.listen(0); + await once(proxy2, 'listening'); + + logs.splice(0, logs.length); + await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `http://${serverHost}/test`, + http_proxy: `http://localhost:${proxy.address().port}`, + HTTP_PROXY: `http://localhost:${proxy2.address().port}`, + }, { + stdout: 'Hello world', + }); + assert.deepStrictEqual(logs, expectedLogs); + proxy2.close(); +} proxy.close(); -proxy2.close(); server.close(); diff --git a/test/client-proxy/test-https-proxy-fetch.mjs b/test/client-proxy/test-https-proxy-fetch.mjs index 8a3d7f07284e9a..34cde73055c72b 100644 --- a/test/client-proxy/test-https-proxy-fetch.mjs +++ b/test/client-proxy/test-https-proxy-fetch.mjs @@ -1,7 +1,6 @@ import * as common from '../common/index.mjs'; import fixtures from '../common/fixtures.js'; import assert from 'node:assert'; -import https from 'node:https'; import http from 'node:http'; import { once } from 'events'; import { createProxyServer, checkProxiedFetch } from '../common/proxy-server.js'; @@ -9,13 +8,17 @@ import { createProxyServer, checkProxiedFetch } from '../common/proxy-server.js' if (!common.hasCrypto) common.skip('missing crypto'); +// https must be dynamically imported so that builds without crypto support +// can skip it. +const https = (await import('node:https')).default; + // Start a server to process the final request. const server = https.createServer({ cert: fixtures.readKey('agent8-cert.pem'), key: fixtures.readKey('agent8-key.pem'), }, common.mustCall((req, res) => { res.end('Hello world'); -}, 3)); +}, common.isWindows ? 2 : 3)); server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); server.listen(0); await once(server, 'listening'); @@ -60,24 +63,29 @@ await checkProxiedFetch({ }); assert.deepStrictEqual(logs, expectedLogs); -const proxy2 = http.createServer(); -proxy2.on('connect', common.mustNotCall()); -proxy2.listen(0); -await once(proxy2, 'listening'); +// Check lower-cased http_proxy environment variable takes precedence. +// On Windows, environment variables are case-insensitive, so this test +// is not applicable. +if (!common.isWindows) { + const proxy2 = http.createServer(); + proxy2.on('connect', common.mustNotCall()); + proxy2.listen(0); + await once(proxy2, 'listening'); + + logs.splice(0, logs.length); + await checkProxiedFetch({ + NODE_USE_ENV_PROXY: 1, + FETCH_URL: `https://${serverHost}/test`, + https_proxy: `http://localhost:${proxy.address().port}`, + HTTPS_PROXY: `http://localhost:${proxy2.address().port}`, + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), + }, { + stdout: 'Hello world', + }); + assert.deepStrictEqual(logs, expectedLogs); + proxy2.close(); +} -// Check lower-cased https_proxy environment variable takes precedence. -logs.splice(0, logs.length); -await checkProxiedFetch({ - NODE_USE_ENV_PROXY: 1, - FETCH_URL: `https://${serverHost}/test`, - https_proxy: `http://localhost:${proxy.address().port}`, - HTTPS_PROXY: `http://localhost:${proxy2.address().port}`, - NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), -}, { - stdout: 'Hello world', -}); -assert.deepStrictEqual(logs, expectedLogs); proxy.close(); -proxy2.close(); server.close();