Skip to content

[core] Upgrade http(s)-proxy-agent #28563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mpodwysocki merged 1 commit into from
Feb 13, 2024
Merged

[core] Upgrade http(s)-proxy-agent #28563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
776 changes: 387 additions & 389 deletions common/config/rush/pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions sdk/core/core-rest-pipeline/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@
"@azure/core-tracing": "^1.0.1",
"@azure/core-util": "^1.3.0",
"@azure/logger": "^1.0.0",
"tslib": "^2.2.0",
"http-proxy-agent": "^5.0.0",
"https-proxy-agent": "^5.0.0"
"http-proxy-agent": "^7.0.1",
"https-proxy-agent": "^7.0.3",
"tslib": "^2.6.2"
},
"devDependencies": {
"@azure/dev-tool": "^1.0.0",
Expand Down
63 changes: 21 additions & 42 deletions sdk/core/core-rest-pipeline/src/policies/proxyPolicy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

import type * as http from "http";
import type * as https from "https";
import { HttpsProxyAgent, type HttpsProxyAgentOptions } from "https-proxy-agent";
import { HttpProxyAgent, type HttpProxyAgentOptions } from "http-proxy-agent";
import { HttpsProxyAgent } from "https-proxy-agent";
import { HttpProxyAgent } from "http-proxy-agent";
import type { PipelineRequest, PipelineResponse, ProxySettings, SendRequest } from "../interfaces";
import type { PipelinePolicy } from "../pipeline";
import { logger } from "../log";
Expand Down Expand Up @@ -126,42 +126,6 @@ export function getDefaultProxySettings(proxyUrl?: string): ProxySettings | unde
};
}

/**
* @internal
*/
export function getProxyAgentOptions(
proxySettings: ProxySettings,
{ headers, tlsSettings }: PipelineRequest,
): HttpProxyAgentOptions {
let parsedProxyUrl: URL;
try {
parsedProxyUrl = new URL(proxySettings.host);
} catch (_error) {
throw new Error(
`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`,
);
}

if (tlsSettings) {
logger.warning(
"TLS settings are not supported in combination with custom Proxy, certificates provided to the client will be ignored.",
);
}

const proxyAgentOptions: HttpsProxyAgentOptions = {
hostname: parsedProxyUrl.hostname,
port: proxySettings.port,
protocol: parsedProxyUrl.protocol,
headers: headers.toJSON(),
};
if (proxySettings.username && proxySettings.password) {
proxyAgentOptions.auth = `${proxySettings.username}:${proxySettings.password}`;
} else if (proxySettings.username) {
proxyAgentOptions.auth = `${proxySettings.username}`;
}
return proxyAgentOptions;
}

function setProxyAgentOnRequest(request: PipelineRequest, cachedAgents: CachedAgents): void {
// Custom Agent should take precedence so if one is present
// we should skip to avoid overwriting it.
Expand All @@ -175,16 +139,31 @@ function setProxyAgentOnRequest(request: PipelineRequest, cachedAgents: CachedAg

const proxySettings = request.proxySettings;
if (proxySettings) {
let parsedProxyUrl: URL;
try {
parsedProxyUrl = new URL(proxySettings.host);
} catch (_error) {
throw new Error(
`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`,
);
}

if (request.tlsSettings) {
logger.warning(
"TLS settings are not supported in combination with custom Proxy, certificates provided to the client will be ignored.",
);
}

const headers = request.headers.toJSON();

if (isInsecure) {
if (!cachedAgents.httpProxyAgent) {
const proxyAgentOptions = getProxyAgentOptions(proxySettings, request);
cachedAgents.httpProxyAgent = new HttpProxyAgent(proxyAgentOptions);
cachedAgents.httpProxyAgent = new HttpProxyAgent(parsedProxyUrl, { headers });
}
request.agent = cachedAgents.httpProxyAgent;
} else {
if (!cachedAgents.httpsProxyAgent) {
const proxyAgentOptions = getProxyAgentOptions(proxySettings, request);
cachedAgents.httpsProxyAgent = new HttpsProxyAgent(proxyAgentOptions);
cachedAgents.httpsProxyAgent = new HttpsProxyAgent(parsedProxyUrl, { headers });
}
request.agent = cachedAgents.httpsProxyAgent;
}
Expand Down
33 changes: 1 addition & 32 deletions sdk/core/core-rest-pipeline/test/node/proxyPolicy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import {
getDefaultProxySettings,
proxyPolicy,
} from "../../src";
import {
getProxyAgentOptions,
globalNoProxyList,
loadNoProxy,
} from "../../src/policies/proxyPolicy";
import { globalNoProxyList, loadNoProxy } from "../../src/policies/proxyPolicy";

describe("proxyPolicy (node)", function () {
it("Sets proxy settings on the request", function () {
Expand Down Expand Up @@ -174,33 +170,6 @@ describe("proxyPolicy (node)", function () {
}
});

it("getProxyAgentOptions from proxy settings having both username and password", function () {
const proxySettings: ProxySettings = {
host: "https://proxy.example.com",
port: 8080,
username: "user",
password: "pass",
};
const options = getProxyAgentOptions(
proxySettings,
createPipelineRequest({ url: "https://example.org" }),
);
assert.strictEqual(options.auth, "user:pass");
});

it("getProxyAgentOptions from proxy settings having username but no password", function () {
const proxySettings: ProxySettings = {
host: "https://proxy.example.com",
port: 8080,
username: "user",
};
const options = getProxyAgentOptions(
proxySettings,
createPipelineRequest({ url: "https://example.org" }),
);
assert.strictEqual(options.auth, "user");
});

describe("getDefaultProxySettings", function () {
it("Parses a url without a port", function () {
const proxyUrl = "https://proxy.example.com";
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/ts-http-runtime/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@
},
"dependencies": {
"tslib": "^2.2.0",
"http-proxy-agent": "^5.0.0",
"https-proxy-agent": "^5.0.0"
"http-proxy-agent": "^7.0.1",
"https-proxy-agent": "^7.0.3"
},
"devDependencies": {
"@azure/dev-tool": "^1.0.0",
Expand Down
71 changes: 25 additions & 46 deletions sdk/core/ts-http-runtime/src/policies/proxyPolicy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import * as http from "http";
import * as https from "https";
import { HttpsProxyAgent, HttpsProxyAgentOptions } from "https-proxy-agent";
import { HttpProxyAgent, HttpProxyAgentOptions } from "http-proxy-agent";
import { PipelineRequest, PipelineResponse, ProxySettings, SendRequest } from "../interfaces";
import { PipelinePolicy } from "../pipeline";
import type * as http from "http";
import type * as https from "https";
import { HttpsProxyAgent } from "https-proxy-agent";
import { HttpProxyAgent } from "http-proxy-agent";
import type { PipelineRequest, PipelineResponse, ProxySettings, SendRequest } from "../interfaces";
import type { PipelinePolicy } from "../pipeline";
import { logger } from "../log";

const HTTPS_PROXY = "HTTPS_PROXY";
Expand Down Expand Up @@ -126,42 +126,6 @@ export function getDefaultProxySettings(proxyUrl?: string): ProxySettings | unde
};
}

/**
* @internal
*/
export function getProxyAgentOptions(
proxySettings: ProxySettings,
{ headers, tlsSettings }: PipelineRequest,
): HttpProxyAgentOptions {
let parsedProxyUrl: URL;
try {
parsedProxyUrl = new URL(proxySettings.host);
} catch (_error) {
throw new Error(
`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`,
);
}

if (tlsSettings) {
logger.warning(
"TLS settings are not supported in combination with custom Proxy, certificates provided to the client will be ignored.",
);
}

const proxyAgentOptions: HttpsProxyAgentOptions = {
hostname: parsedProxyUrl.hostname,
port: proxySettings.port,
protocol: parsedProxyUrl.protocol,
headers: headers.toJSON(),
};
if (proxySettings.username && proxySettings.password) {
proxyAgentOptions.auth = `${proxySettings.username}:${proxySettings.password}`;
} else if (proxySettings.username) {
proxyAgentOptions.auth = `${proxySettings.username}`;
}
return proxyAgentOptions;
}

function setProxyAgentOnRequest(request: PipelineRequest, cachedAgents: CachedAgents): void {
// Custom Agent should take precedence so if one is present
// we should skip to avoid overwriting it.
Expand All @@ -175,16 +139,31 @@ function setProxyAgentOnRequest(request: PipelineRequest, cachedAgents: CachedAg

const proxySettings = request.proxySettings;
if (proxySettings) {
let parsedProxyUrl: URL;
try {
parsedProxyUrl = new URL(proxySettings.host);
} catch (_error) {
throw new Error(
`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`,
);
}

if (request.tlsSettings) {
logger.warning(
"TLS settings are not supported in combination with custom Proxy, certificates provided to the client will be ignored.",
);
}

const headers = request.headers.toJSON();

if (isInsecure) {
if (!cachedAgents.httpProxyAgent) {
const proxyAgentOptions = getProxyAgentOptions(proxySettings, request);
cachedAgents.httpProxyAgent = new HttpProxyAgent(proxyAgentOptions);
cachedAgents.httpProxyAgent = new HttpProxyAgent(parsedProxyUrl, { headers });
}
request.agent = cachedAgents.httpProxyAgent;
} else {
if (!cachedAgents.httpsProxyAgent) {
const proxyAgentOptions = getProxyAgentOptions(proxySettings, request);
cachedAgents.httpsProxyAgent = new HttpsProxyAgent(proxyAgentOptions);
cachedAgents.httpsProxyAgent = new HttpsProxyAgent(parsedProxyUrl, { headers });
}
request.agent = cachedAgents.httpsProxyAgent;
}
Expand Down
33 changes: 1 addition & 32 deletions sdk/core/ts-http-runtime/test/node/proxyPolicy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import {
getDefaultProxySettings,
proxyPolicy,
} from "../../src";
import {
getProxyAgentOptions,
globalNoProxyList,
loadNoProxy,
} from "../../src/policies/proxyPolicy";
import { globalNoProxyList, loadNoProxy } from "../../src/policies/proxyPolicy";

describe("proxyPolicy (node)", function () {
it("Sets proxy settings on the request", function () {
Expand Down Expand Up @@ -174,33 +170,6 @@ describe("proxyPolicy (node)", function () {
}
});

it("getProxyAgentOptions from proxy settings having both username and password", function () {
const proxySettings: ProxySettings = {
host: "https://proxy.example.com",
port: 8080,
username: "user",
password: "pass",
};
const options = getProxyAgentOptions(
proxySettings,
createPipelineRequest({ url: "https://example.org" }),
);
assert.strictEqual(options.auth, "user:pass");
});

it("getProxyAgentOptions from proxy settings having username but no password", function () {
const proxySettings: ProxySettings = {
host: "https://proxy.example.com",
port: 8080,
username: "user",
};
const options = getProxyAgentOptions(
proxySettings,
createPipelineRequest({ url: "https://example.org" }),
);
assert.strictEqual(options.auth, "user");
});

describe("getDefaultProxySettings", function () {
it("Parses a url without a port", function () {
const proxyUrl = "https://proxy.example.com";
Expand Down