Skip to content

Commit

Permalink
[core-http] validate proxySettings when creating proxy agent (#7149)
Browse files Browse the repository at this point in the history
* [core-http] validate proxySettings when creating proxy agent

Throw separate errors for invalid host and invalid port

Resolves #4544.
  • Loading branch information
jeremymeng authored Jan 31, 2020
1 parent 2875845 commit 024e4d4
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 1 deletion.
19 changes: 18 additions & 1 deletion sdk/core/core-http/lib/proxyAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,20 @@ export function createProxyAgent(
proxySettings: ProxySettings,
headers?: HttpHeaders
): ProxyAgent {
const host = URLBuilder.parse(proxySettings.host).getHost() as string;
if (!host) {
throw new Error(
"Expecting a non-empty host in proxy settings."
);
}
if (!isValidPort(proxySettings.port)) {
throw new Error(
"Expecting a valid port number in the range of [0, 65535] in proxy settings."
);
}
const tunnelOptions: tunnel.HttpsOverHttpsOptions = {
proxy: {
host: URLBuilder.parse(proxySettings.host).getHost() as string,
host: host,
port: proxySettings.port,
headers: (headers && headers.rawHeaders()) || {}
}
Expand Down Expand Up @@ -58,3 +69,9 @@ export function createTunnel(
return tunnel.httpOverHttp(tunnelOptions);
}
}

function isValidPort(port: number) {
// any port in 0-65535 range is valid (RFC 793) even though almost all implementations
// will reserve 0 for a specific purpose, and a range of numbers for ephemeral ports
return 0 <= port && port <= 65535;
}
41 changes: 41 additions & 0 deletions sdk/core/core-http/test/proxyAgent.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,47 @@ describe("proxyAgent", () => {
agent.proxyOptions.headers!.should.contain({ "user-agent": "Node.js" });
done();
});

[
{ host: "host", port: 0 },
{ host: "host", port: 65535 }
].forEach((testCase) => {
it(`should not throw error when being given a valid proxy settings of { host: '${testCase.host}', port: ${testCase.port} }.`, function(done) {
const proxySettings = {
host: testCase.host,
port: testCase.port
};

const fn = function() {
createProxyAgent("http://example.com", proxySettings);
};
fn.should.not.throw();
done();
});
});

[
{ host: "", port: 8080, expectInvalidHostError: true },
{ host: "host", port: -1, expectInvalidHostError: false },
{ host: "host", port: 65536, expectInvalidHostError: false }
].forEach((testCase) => {
it(`should throw error when being given an invalid proxy settings of { host: '${testCase.host}', port: ${testCase.port} }.`, function(done) {
const proxySettings = {
host: testCase.host,
port: testCase.port
};

const fn = function() {
createProxyAgent("http://example.com", proxySettings);
};
fn.should.throw(
testCase.expectInvalidHostError?
"Expecting a non-empty host in proxy settings." :
"Expecting a valid port number in the range of [0, 65535] in proxy settings."
);
done();
});
});
});

describe("createTunnel", () => {
Expand Down

0 comments on commit 024e4d4

Please sign in to comment.