Skip to content

Commit 26e0f77

Browse files
committed
fixup! http,https: add built-in proxy support in http/https.request and Agent
1 parent 16602ae commit 26e0f77

File tree

5 files changed

+114
-0
lines changed

5 files changed

+114
-0
lines changed

doc/api/errors.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2489,6 +2489,12 @@ Accessing `Object.prototype.__proto__` has been forbidden using
24892489
[`Object.setPrototypeOf`][] should be used to get and set the prototype of an
24902490
object.
24912491

2492+
<a id="ERR_PROXY_INVALID_CONFIG"></a>
2493+
2494+
### `ERR_PROXY_INVALID_CONFIG`
2495+
2496+
Failed to proxy a request because the proxy configuration is invalid.
2497+
24922498
<a id="ERR_PROXY_TUNNEL"></a>
24932499

24942500
### `ERR_PROXY_TUNNEL`

lib/internal/errors.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,6 +1670,7 @@ E('ERR_PERFORMANCE_INVALID_TIMESTAMP',
16701670
'%d is not a valid timestamp', TypeError);
16711671
E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS', '%s', TypeError);
16721672
E('ERR_PROXY_TUNNEL', '%s', Error);
1673+
E('ERR_PROXY_INVALID_CONFIG', '%s', Error);
16731674
E('ERR_QUIC_APPLICATION_ERROR', 'A QUIC application error occurred. %d [%s]', Error);
16741675
E('ERR_QUIC_CONNECTION_FAILED', 'QUIC connection failed', Error);
16751676
E('ERR_QUIC_ENDPOINT_CLOSED', 'QUIC endpoint closed: %s (%d)', Error);

lib/internal/http.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const {
1717
const { URL } = require('internal/url');
1818
const { Buffer } = require('buffer');
1919
const { isIPv4 } = require('internal/net');
20+
const { ERR_PROXY_INVALID_CONFIG } = require('internal/errors').codes;
2021
let utcCache;
2122

2223
function utcDate() {
@@ -185,13 +186,18 @@ function parseProxyConfigFromEnv(env, protocol, keepAlive) {
185186
return null;
186187
}
187188
// Get the proxy url - following the most popular convention, lower case takes precedence.
189+
// See https://about.gitlab.com/blog/we-need-to-talk-no-proxy/#http_proxy-and-https_proxy
188190
const proxyUrl = (protocol === 'https:') ?
189191
(env.https_proxy || env.HTTPS_PROXY) : (env.http_proxy || env.HTTP_PROXY);
190192
// No proxy settings from the environment, ignore.
191193
if (!proxyUrl) {
192194
return null;
193195
}
194196

197+
if (proxyUrl.includes('\r') || proxyUrl.includes('\n')) {
198+
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
199+
}
200+
195201
// Only http:// and https:// proxies are supported.
196202
// Ignore instead of throw, in case other protocols are supposed to be
197203
// handled by the user land.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// This tests that proxy URLs with invalid credentials (containing \r or \n characters)
2+
// are rejected with an appropriate error.
3+
import assert from 'node:assert';
4+
import http from 'node:http';
5+
6+
const testCases = [
7+
{
8+
name: 'carriage return in username',
9+
proxyUrl: 'http://user\r:pass@proxy.example.com:8080',
10+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
11+
},
12+
{
13+
name: 'newline in username',
14+
proxyUrl: 'http://user\n:pass@proxy.example.com:8080',
15+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
16+
},
17+
{
18+
name: 'carriage return in password',
19+
proxyUrl: 'http://user:pass\r@proxy.example.com:8080',
20+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
21+
},
22+
{
23+
name: 'newline in password',
24+
proxyUrl: 'http://user:pass\n@proxy.example.com:8080',
25+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
26+
},
27+
{
28+
name: 'CRLF injection attempt in username',
29+
proxyUrl: 'http://user\r\nHost: example.com:pass@proxy.example.com:8080',
30+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
31+
},
32+
{
33+
name: 'CRLF injection attempt in password',
34+
proxyUrl: 'http://user:pass\r\nHost: example.com@proxy.example.com:8080',
35+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
36+
},
37+
];
38+
39+
for (const testCase of testCases) {
40+
// Test that creating an agent with invalid proxy credentials throws an error
41+
assert.throws(() => {
42+
new http.Agent({
43+
proxyEnv: {
44+
HTTP_PROXY: testCase.proxyUrl,
45+
},
46+
});
47+
}, testCase.expectedError);
48+
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// This tests that HTTPS proxy URLs with invalid credentials (containing \r or \n characters)
2+
// are rejected with an appropriate error.
3+
import * as common from '../common/index.mjs';
4+
import assert from 'node:assert';
5+
import https from 'node:https';
6+
7+
if (!common.hasCrypto) {
8+
common.skip('missing crypto');
9+
}
10+
11+
const testCases = [
12+
{
13+
name: 'carriage return in username (HTTPS)',
14+
proxyUrl: 'https://user\r:pass@proxy.example.com:8080',
15+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
16+
},
17+
{
18+
name: 'newline in username (HTTPS)',
19+
proxyUrl: 'https://user\n:pass@proxy.example.com:8080',
20+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
21+
},
22+
{
23+
name: 'carriage return in password (HTTPS)',
24+
proxyUrl: 'https://user:pass\r@proxy.example.com:8080',
25+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
26+
},
27+
{
28+
name: 'newline in password (HTTPS)',
29+
proxyUrl: 'https://user:pass\n@proxy.example.com:8080',
30+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
31+
},
32+
{
33+
name: 'CRLF injection attempt in username (HTTPS)',
34+
proxyUrl: 'https://user\r\nHost: example.com:pass@proxy.example.com:8080',
35+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
36+
},
37+
{
38+
name: 'CRLF injection attempt in password (HTTPS)',
39+
proxyUrl: 'https://user:pass\r\nHost: example.com@proxy.example.com:8080',
40+
expectedError: { code: 'ERR_PROXY_INVALID_CONFIG', message: /Invalid proxy URL/ },
41+
},
42+
];
43+
44+
for (const testCase of testCases) {
45+
// Test that creating an agent with invalid proxy credentials throws an error
46+
assert.throws(() => {
47+
new https.Agent({
48+
proxyEnv: {
49+
HTTPS_PROXY: testCase.proxyUrl,
50+
},
51+
});
52+
}, testCase.expectedError);
53+
}

0 commit comments

Comments
 (0)