Skip to content

Commit

Permalink
fix: correct CONNECT proxy handling and needle upgrade (#4967)
Browse files Browse the repository at this point in the history
* fix: fix the proxy tests, ensure that CONNECT method is used

* chore: upgrade needle to 3.3.0

Expect tests to fail / CONNECT will not be used until proxy is disabled in needle.

* fix: disable proxy in needle, global-agent will be responsible to proxy requests

* fix: upgrade @types/needle to support use_proxy_from_env_var

* fix: upgrade @snyk/code-client
  • Loading branch information
neonnoon committed Jan 8, 2024
1 parent f3b705f commit 0226e20
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 105 deletions.
140 changes: 40 additions & 100 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Expand Up @@ -63,7 +63,7 @@
"@sentry/node": "^7.34.0",
"@snyk/cli-interface": "2.12.0",
"@snyk/cloud-config-parser": "^1.14.5",
"@snyk/code-client": "^4.23.3",
"@snyk/code-client": "^4.23.5",
"@snyk/dep-graph": "^2.7.4",
"@snyk/docker-registry-v2-client": "^2.11.0",
"@snyk/fix": "file:packages/snyk-fix",
Expand Down Expand Up @@ -104,7 +104,7 @@
"lodash.values": "^4.3.0",
"marked": "^4.0.1",
"micromatch": "4.0.2",
"needle": "3.0.0",
"needle": "^3.3.0",
"open": "^7.0.3",
"ora": "5.4.0",
"os-name": "^5.1.0",
Expand Down Expand Up @@ -139,7 +139,7 @@
"@types/fs-extra": "^9.0.11",
"@types/jest": "^27.0.1",
"@types/lodash": "^4.14.161",
"@types/needle": "^2.0.4",
"@types/needle": "^3.3.0",
"@types/node": "^14.14.31",
"@types/sarif": "^2.1.2",
"@types/sinon": "^7.5.0",
Expand Down
1 change: 1 addition & 0 deletions src/lib/request/request.ts
Expand Up @@ -107,6 +107,7 @@ function setupRequest(payload: Payload) {
? new http.Agent({ keepAlive: true })
: new https.Agent({ keepAlive: true });
const options: needle.NeedleOptions = {
use_proxy_from_env_var: false,
json: payload.json,
parse: payload.parse,
headers: payload.headers,
Expand Down
10 changes: 8 additions & 2 deletions test/tap/proxy.test.js
Expand Up @@ -73,6 +73,8 @@ test('request respects proxy environment variables', async (t) => {
});

t.test('https_proxy', async (t) => {
t.plan(3);

// NO_PROXY is set in CircleCI and brakes test purpose
const tmpNoProxy = process.env.NO_PROXY;
delete process.env.NO_PROXY;
Expand All @@ -89,6 +91,7 @@ test('request respects proxy environment variables', async (t) => {
proxy.setTimeout(1000);
proxy.on('connect', (req, cltSocket) => {
const proxiedUrl = url.parse(`https://${req.url}`);
t.equal(req.method, 'CONNECT', 'Proxy for HTTPS using CONNECT');
t.equal(
proxiedUrl.hostname,
url.parse(httpsRequestHost).hostname,
Expand All @@ -114,7 +117,7 @@ test('request respects proxy environment variables', async (t) => {
try {
await makeRequest({
method: 'post',
url: httpRequestHost + requestPath,
url: httpsRequestHost + requestPath,
});
} catch (e) {
// an exception is expected
Expand All @@ -123,6 +126,8 @@ test('request respects proxy environment variables', async (t) => {
});

t.test('HTTPS_PROXY', async (t) => {
t.plan(3);

// NO_PROXY is set in CircleCI and brakes test purpose
const tmpNoProxy = process.env.NO_PROXY;
delete process.env.NO_PROXY;
Expand All @@ -138,6 +143,7 @@ test('request respects proxy environment variables', async (t) => {
proxy.setTimeout(1000);
proxy.on('connect', (req, cltSocket) => {
const proxiedUrl = url.parse(`https://${req.url}`);
t.equal(req.method, 'CONNECT', 'Proxy for HTTPS using CONNECT');
t.equal(
proxiedUrl.hostname,
url.parse(httpsRequestHost).hostname,
Expand All @@ -163,7 +169,7 @@ test('request respects proxy environment variables', async (t) => {
try {
await makeRequest({
method: 'post',
url: httpRequestHost + requestPath,
url: httpsRequestHost + requestPath,
});
} catch (e) {
// an exception is expected
Expand Down

0 comments on commit 0226e20

Please sign in to comment.