Skip to content

Commit ebec3ef

Browse files
joyeecheungaduh95
authored andcommitted
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 PR-URL: #58980 Refs: #57872 Refs: #8381 Refs: #15620 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent bedaaa1 commit ebec3ef

7 files changed

Lines changed: 255 additions & 135 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
prefix client-proxy
2+
3+
# To mark a test as flaky, list the test name in the appropriate section
4+
# below, without ".js", followed by ": PASS,FLAKY". Example:
5+
# sample-test : PASS,FLAKY
6+
7+
[true] # This section applies to all platforms
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import*ascommonfrom'../common/index.mjs';
2+
importassertfrom'node:assert';
3+
import{once}from'events';
4+
importhttpfrom'node:http';
5+
import{createProxyServer,checkProxiedFetch}from'../common/proxy-server.js';
6+
7+
// Start a server to process the final request.
8+
constserver=http.createServer(common.mustCall((req,res)=>{
9+
res.end('Hello world');
10+
},common.isWindows ? 2 : 3));
11+
server.on('error',common.mustNotCall((err)=>{console.error('Server error',err);}));
12+
server.listen(0);
13+
awaitonce(server,'listening');
14+
15+
// Start a minimal proxy server.
16+
const{ proxy, logs }=createProxyServer();
17+
proxy.listen(0);
18+
awaitonce(proxy,'listening');
19+
20+
constserverHost=`localhost:${server.address().port}`;
21+
22+
// FIXME(undici:4083): undici currently always tunnels the request over
23+
// CONNECT if proxyTunnel is not explicitly set to false, but what we
24+
// need is for it to be automatically false for HTTP requests to be
25+
// consistent with curl.
26+
constexpectedLogs=[{
27+
method: 'CONNECT',
28+
url: serverHost,
29+
headers: {
30+
// FIXME(undici:4086): this should be keep-alive.
31+
connection: 'close',
32+
host: serverHost,
33+
},
34+
}];
35+
36+
// Check upper-cased HTTPS_PROXY environment variable.
37+
awaitcheckProxiedFetch({
38+
NODE_USE_ENV_PROXY: 1,
39+
FETCH_URL: `http://${serverHost}/test`,
40+
HTTP_PROXY: `http://localhost:${proxy.address().port}`,
41+
},{
42+
stdout: 'Hello world',
43+
});
44+
assert.deepStrictEqual(logs,expectedLogs);
45+
46+
// Check lower-cased https_proxy environment variable.
47+
logs.splice(0,logs.length);
48+
awaitcheckProxiedFetch({
49+
NODE_USE_ENV_PROXY: 1,
50+
FETCH_URL: `http://${serverHost}/test`,
51+
http_proxy: `http://localhost:${proxy.address().port}`,
52+
},{
53+
stdout: 'Hello world',
54+
});
55+
assert.deepStrictEqual(logs,expectedLogs);
56+
57+
// Check lower-cased http_proxy environment variable takes precedence.
58+
// On Windows, environment variables are case-insensitive, so this test
59+
// is not applicable.
60+
if(!common.isWindows){
61+
constproxy2=http.createServer();
62+
proxy2.on('connect',common.mustNotCall());
63+
proxy2.listen(0);
64+
awaitonce(proxy2,'listening');
65+
66+
logs.splice(0,logs.length);
67+
awaitcheckProxiedFetch({
68+
NODE_USE_ENV_PROXY: 1,
69+
FETCH_URL: `http://${serverHost}/test`,
70+
http_proxy: `http://localhost:${proxy.address().port}`,
71+
HTTP_PROXY: `http://localhost:${proxy2.address().port}`,
72+
},{
73+
stdout: 'Hello world',
74+
});
75+
assert.deepStrictEqual(logs,expectedLogs);
76+
proxy2.close();
77+
}
78+
79+
proxy.close();
80+
server.close();
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import*ascommonfrom'../common/index.mjs';
2+
importfixturesfrom'../common/fixtures.js';
3+
importassertfrom'node:assert';
4+
importhttpfrom'node:http';
5+
import{once}from'events';
6+
import{createProxyServer,checkProxiedFetch}from'../common/proxy-server.js';
7+
8+
if(!common.hasCrypto)
9+
common.skip('missing crypto');
10+
11+
// https must be dynamically imported so that builds without crypto support
12+
// can skip it.
13+
consthttps=(awaitimport('node:https')).default;
14+
15+
// Start a server to process the final request.
16+
constserver=https.createServer({
17+
cert: fixtures.readKey('agent8-cert.pem'),
18+
key: fixtures.readKey('agent8-key.pem'),
19+
},common.mustCall((req,res)=>{
20+
res.end('Hello world');
21+
},common.isWindows ? 2 : 3));
22+
server.on('error',common.mustNotCall((err)=>{console.error('Server error',err);}));
23+
server.listen(0);
24+
awaitonce(server,'listening');
25+
26+
// Start a minimal proxy server.
27+
const{ proxy, logs }=createProxyServer();
28+
proxy.listen(0);
29+
awaitonce(proxy,'listening');
30+
31+
constserverHost=`localhost:${server.address().port}`;
32+
33+
constexpectedLogs=[{
34+
method: 'CONNECT',
35+
url: serverHost,
36+
headers: {
37+
// FIXME(undici:4086): this should be keep-alive.
38+
connection: 'close',
39+
host: serverHost,
40+
},
41+
}];
42+
43+
// Check upper-cased HTTPS_PROXY environment variable.
44+
awaitcheckProxiedFetch({
45+
NODE_USE_ENV_PROXY: 1,
46+
FETCH_URL: `https://${serverHost}/test`,
47+
HTTPS_PROXY: `http://localhost:${proxy.address().port}`,
48+
NODE_EXTRA_CA_CERTS: fixtures.path('keys','fake-startcom-root-cert.pem'),
49+
},{
50+
stdout: 'Hello world',
51+
});
52+
assert.deepStrictEqual(logs,expectedLogs);
53+
54+
// Check lower-cased https_proxy environment variable.
55+
logs.splice(0,logs.length);
56+
awaitcheckProxiedFetch({
57+
NODE_USE_ENV_PROXY: 1,
58+
FETCH_URL: `https://${serverHost}/test`,
59+
https_proxy: `http://localhost:${proxy.address().port}`,
60+
NODE_EXTRA_CA_CERTS: fixtures.path('keys','fake-startcom-root-cert.pem'),
61+
},{
62+
stdout: 'Hello world',
63+
});
64+
assert.deepStrictEqual(logs,expectedLogs);
65+
66+
// Check lower-cased http_proxy environment variable takes precedence.
67+
// On Windows, environment variables are case-insensitive, so this test
68+
// is not applicable.
69+
if(!common.isWindows){
70+
constproxy2=http.createServer();
71+
proxy2.on('connect',common.mustNotCall());
72+
proxy2.listen(0);
73+
awaitonce(proxy2,'listening');
74+
75+
logs.splice(0,logs.length);
76+
awaitcheckProxiedFetch({
77+
NODE_USE_ENV_PROXY: 1,
78+
FETCH_URL: `https://${serverHost}/test`,
79+
https_proxy: `http://localhost:${proxy.address().port}`,
80+
HTTPS_PROXY: `http://localhost:${proxy2.address().port}`,
81+
NODE_EXTRA_CA_CERTS: fixtures.path('keys','fake-startcom-root-cert.pem'),
82+
},{
83+
stdout: 'Hello world',
84+
});
85+
assert.deepStrictEqual(logs,expectedLogs);
86+
proxy2.close();
87+
}
88+
89+
90+
proxy.close();
91+
server.close();

‎test/client-proxy/testcfg.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
importsys, os
2+
sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
3+
importtestpy
4+
5+
defGetConfiguration(context, root):
6+
returntestpy.ParallelTestConfiguration(context, root, 'client-proxy')

‎test/common/proxy-server.js‎

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,32 @@ function logRequest(logs, req) {
1414

1515
// This creates a minimal proxy server that logs the requests it gets
1616
// to an array before performing proxying.
17-
exports.createProxyServer=function(){
17+
exports.createProxyServer=function(options={}){
1818
constlogs=[];
1919

20-
constproxy=http.createServer();
20+
letproxy;
21+
if(options.https){
22+
constcommon=require('../common');
23+
if(!common.hasCrypto){
24+
common.skip('missing crypto');
25+
}
26+
proxy=require('https').createServer({
27+
cert: require('./fixtures').readKey('agent9-cert.pem'),
28+
key: require('./fixtures').readKey('agent9-key.pem'),
29+
});
30+
}else{
31+
proxy=http.createServer();
32+
}
2133
proxy.on('request',(req,res)=>{
2234
logRequest(logs,req);
2335
const[hostname,port]=req.headers.host.split(':');
2436
consttargetPort=port||80;
2537

38+
consturl=newURL(req.url);
2639
constoptions={
2740
hostname: hostname,
2841
port: targetPort,
29-
path: req.url,
42+
path: url.pathname+url.search,// Convert back to relative URL.
3043
method: req.method,
3144
headers: req.headers,
3245
};
@@ -38,8 +51,16 @@ exports.createProxyServer = function() {
3851

3952
proxyReq.on('error',(err)=>{
4053
logs.push({error: err,source: 'proxy request'});
41-
res.writeHead(500);
42-
res.end('Proxy error: '+err.message);
54+
if(!res.headersSent){
55+
res.writeHead(500);
56+
}
57+
if(!res.writableEnded){
58+
res.end(`Proxy error ${err.code}: ${err.message}`);
59+
}
60+
});
61+
62+
res.on('error',(err)=>{
63+
logs.push({error: err,source: 'proxy response'});
4364
});
4465

4566
req.pipe(proxyReq,{end: true});
@@ -49,6 +70,11 @@ exports.createProxyServer = function() {
4970
logRequest(logs,req);
5071

5172
const[hostname,port]=req.url.split(':');
73+
74+
res.on('error',(err)=>{
75+
logs.push({error: err,source: 'proxy response'});
76+
});
77+
5278
constproxyReq=net.connect(port,hostname,()=>{
5379
res.write(
5480
'HTTP/1.1 200 Connection Established\r\n'+
@@ -74,8 +100,46 @@ exports.createProxyServer = function() {
74100
return{ proxy, logs };
75101
};
76102

77-
exports.checkProxiedRequest=asyncfunction(envExtension,expectation){
78-
const{ spawnPromisified }=require('./');
103+
functionspawnPromisified(...args){
104+
const{ spawn }=require('child_process');
105+
letstderr='';
106+
letstdout='';
107+
108+
constchild=spawn(...args);
109+
child.stderr.setEncoding('utf8');
110+
child.stderr.on('data',(data)=>{
111+
console.error('[STDERR]',data);
112+
stderr+=data;
113+
});
114+
child.stdout.setEncoding('utf8');
115+
child.stdout.on('data',(data)=>{
116+
console.log('[STDOUT]',data);
117+
stdout+=data;
118+
});
119+
120+
returnnewPromise((resolve,reject)=>{
121+
child.on('close',(code,signal)=>{
122+
console.log('[CLOSE]',code,signal);
123+
resolve({
124+
code,
125+
signal,
126+
stderr,
127+
stdout,
128+
});
129+
});
130+
child.on('error',(code,signal)=>{
131+
console.log('[ERROR]',code,signal);
132+
reject({
133+
code,
134+
signal,
135+
stderr,
136+
stdout,
137+
});
138+
});
139+
});
140+
}
141+
142+
exports.checkProxiedFetch=asyncfunction(envExtension,expectation){
79143
constfixtures=require('./fixtures');
80144
const{ code, signal, stdout, stderr }=awaitspawnPromisified(
81145
process.execPath,

‎test/parallel/test-http-proxy-fetch.js‎

Lines changed: 0 additions & 61 deletions
This file was deleted.

0 commit comments

Comments
 (0)