Skip to content

Commit

Permalink
Fix including proxy policy in browser (#328)
Browse files Browse the repository at this point in the history 
* Fix including proxy policy in browser
* Add browser tests
  • Loading branch information
@kpajdzik
kpajdzik authored Jan 30, 2019
1 parent c1742fe commit fafa261
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 91 deletions.
32 changes: 32 additions & 0 deletions lib/policies/proxyPolicy.browser.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

import { ProxySettings } from "../serviceClient";
import { BaseRequestPolicy, RequestPolicy, RequestPolicyFactory, RequestPolicyOptions } from "./requestPolicy";
import { HttpOperationResponse } from "../httpOperationResponse";
import { WebResource } from "../webResource";

const proxyNotSupportedInBrowser = new Error("ProxyPolicy is not supported in browser environment");

export function getDefaultProxySettings(_proxyUrl?: string): ProxySettings | undefined {
return undefined;
}

export function proxyPolicy(_proxySettings?: ProxySettings): RequestPolicyFactory {
return {
create: (_nextPolicy: RequestPolicy, _options: RequestPolicyOptions) => {
throw proxyNotSupportedInBrowser;
}
};
}

export class ProxyPolicy extends BaseRequestPolicy {
constructor(nextPolicy: RequestPolicy, options: RequestPolicyOptions) {
super(nextPolicy, options);
throw proxyNotSupportedInBrowser;
}

public sendRequest(_request: WebResource): Promise<HttpOperationResponse> {
throw proxyNotSupportedInBrowser;
}
}
2 changes: 1 addition & 1 deletion lib/util/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const Constants = {
* @const
* @type {string}
*/
msRestVersion: "1.5.3",
msRestVersion: "1.6.0",

/**
* Specifies HTTP.
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"email": "azsdkteam@microsoft.com",
"url": "https://github.com/Azure/ms-rest-js"
},
"version": "1.5.3",
"version": "1.6.0",
"description": "Isomorphic client Runtime for Typescript/node.js/browser javascript client libraries generated using AutoRest",
"tags": [
"isomorphic",
Expand Down
3 changes: 2 additions & 1 deletion rollup.config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@
plugins: [
alias({
"./defaultHttpClient": "./defaultHttpClient.browser",
"./msRestUserAgentPolicy": "./msRestUserAgentPolicy.browser",
"./policies/msRestUserAgentPolicy": "./policies/msRestUserAgentPolicy.browser",
"./policies/proxyPolicy": "./policies/proxyPolicy.browser",
"./util/xml": "./util/xml.browser",
"./util/base64": "./util/base64.browser",
}),
Expand Down
7 changes: 7 additions & 0 deletions test/msAssert.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

import { assert } from "chai";
import { SuiteFunction, PendingSuiteFunction, TestFunction, PendingTestFunction } from "mocha";
import { isNode } from "../lib/util/utils";

export const nodeIt: TestFunction | PendingTestFunction = (!isNode ? it.skip : it);
export const browserIt: TestFunction | PendingTestFunction = (isNode ? it.skip : it);
export const nodeDescribe: SuiteFunction | PendingSuiteFunction = (!isNode ? describe.skip : describe);
export const browserDescribe: SuiteFunction | PendingSuiteFunction = (isNode ? describe.skip : describe);

/**
* Assert that the provided syncFunction throws an Error. If the expectedError is undefined, then
Expand Down
9 changes: 3 additions & 6 deletions test/msRestUserAgentPolicyTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

import "chai/register-should";
import { SuiteFunction, PendingSuiteFunction } from "mocha";

import { HttpOperationResponse } from "../lib/httpOperationResponse";
import { RequestPolicy, RequestPolicyOptions } from "../lib/policies/requestPolicy";
import { Constants } from "../lib/util/constants";
import { WebResource } from "../lib/webResource";
import { userAgentPolicy } from "../lib/policies/userAgentPolicy";
import { isNode } from "../lib/util/utils";
import { nodeDescribe, browserDescribe } from "./msAssert";

const userAgentHeaderKey = Constants.HeaderConstants.USER_AGENT;
export const browserDescribe: SuiteFunction | PendingSuiteFunction = (isNode ? describe.skip : describe);
const nodeDescribe: SuiteFunction | PendingSuiteFunction = (!isNode ? describe.skip : describe);

const emptyRequestPolicy: RequestPolicy = {
sendRequest(request: WebResource): Promise<HttpOperationResponse> {
Expand All @@ -36,7 +33,7 @@ const getUserAgent = async (headerValue?: string): Promise<string> => {
};

describe("MsRestUserAgentPolicy", () => {
nodeDescribe("NodeJS", () => {
nodeDescribe("for Node.js", () => {
it("should not modify user agent header if already present", async () => {
const userAgentPolicy = getPlainUserAgentPolicy();
const customUserAgent = "my custom user agent";
Expand Down Expand Up @@ -99,7 +96,7 @@ describe("MsRestUserAgentPolicy", () => {
});
});

browserDescribe("Browser", function() {
browserDescribe("for browser", function() {
const userAgentHeaderKey = "x-ms-command-name";

const emptyRequestPolicy: RequestPolicy = {
Expand Down
182 changes: 101 additions & 81 deletions test/policies/proxyPolicyTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { WebResource } from "../../lib/webResource";
import { HttpHeaders } from "../../lib/httpHeaders";
import { proxyPolicy, ProxyPolicy, getDefaultProxySettings } from "../../lib/policies/proxyPolicy";
import { Constants } from "../../lib/msRest";
import { nodeDescribe, browserDescribe } from "../msAssert";

describe("ProxyPolicy", function () {
const proxySettings: ProxySettings = {
Expand All @@ -28,124 +29,143 @@ describe("ProxyPolicy", function () {

const emptyPolicyOptions = new RequestPolicyOptions();

it("factory passes correct proxy settings", function (done) {
const factory = proxyPolicy(proxySettings);
const policy = factory.create(emptyRequestPolicy, emptyPolicyOptions) as ProxyPolicy;
nodeDescribe("for Node.js", function () {
it("factory passes correct proxy settings", function (done) {
const factory = proxyPolicy(proxySettings);
const policy = factory.create(emptyRequestPolicy, emptyPolicyOptions) as ProxyPolicy;

policy.proxySettings.should.be.deep.equal(proxySettings);
done();
});
policy.proxySettings.should.be.deep.equal(proxySettings);
done();
});


it("sets correct proxy settings through constructor", function (done) {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
policy.proxySettings.should.be.deep.equal(proxySettings);
done();
});
it("sets correct proxy settings through constructor", function (done) {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
policy.proxySettings.should.be.deep.equal(proxySettings);
done();
});

it("should assign proxy settings to the web request", async () => {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
const request = new WebResource();
it("should assign proxy settings to the web request", async () => {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
const request = new WebResource();

await policy.sendRequest(request);
await policy.sendRequest(request);

request.proxySettings!.should.be.deep.equal(proxySettings);
});
request.proxySettings!.should.be.deep.equal(proxySettings);
});

it("should not override proxy settings to the web request", async () => {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
const request = new WebResource();
const requestSpecificProxySettings = { host: "http://overriden.host", port: 80 };
request.proxySettings = requestSpecificProxySettings;
it("should not override proxy settings to the web request", async () => {
const policy = new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
const request = new WebResource();
const requestSpecificProxySettings = { host: "http://overriden.host", port: 80 };
request.proxySettings = requestSpecificProxySettings;

await policy.sendRequest(request);
await policy.sendRequest(request);

request.proxySettings!.should.be.deep.equal(requestSpecificProxySettings);
request.proxySettings!.should.be.deep.equal(requestSpecificProxySettings);
});
});

browserDescribe("for browser", () => {
it("should throw an Error while constructing object", () => {
const construct = () => new ProxyPolicy(emptyRequestPolicy, emptyPolicyOptions, proxySettings);
construct.should.throw();
});
});
});

describe("getDefaultProxySettings", () => {
const proxyUrl = "https://proxy.microsoft.com";
const defaultPort = 80;

it("should return settings with passed address", () => {
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrl)!;
proxySettings.host.should.equal(proxyUrl);
});

it("should return settings with default port", () => {
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrl)!;
proxySettings.port.should.equal(defaultPort);
});

it("should return settings with passed port", () => {
const port = 3030;
const proxyUrl = "prot://proxy.microsoft.com";
const proxyUrlWithPort = `${proxyUrl}:${port}`;
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrlWithPort)!;
proxySettings.host.should.equal(proxyUrl);
proxySettings.port.should.equal(port);
});

describe("with loadEnvironmentProxyValue", () => {
beforeEach(() => {
delete process.env[Constants.HTTP_PROXY];
delete process.env[Constants.HTTPS_PROXY];
delete process.env[Constants.HTTP_PROXY.toLowerCase()];
delete process.env[Constants.HTTPS_PROXY.toLowerCase()];
nodeDescribe("for Node.js", function () {
it("should return settings with passed address", () => {
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrl)!;
proxySettings.host.should.equal(proxyUrl);
});

it("should return undefined when no proxy passed and environment variable is not set", () => {
const proxySettings: ProxySettings | undefined = getDefaultProxySettings();
should().not.exist(proxySettings);
it("should return settings with default port", () => {
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrl)!;
proxySettings.port.should.equal(defaultPort);
});

it("should load settings from environment variables when no proxyUrl passed", () => {
const proxyUrl = "http://proxy.azure.com";
process.env[Constants.HTTP_PROXY] = proxyUrl;
const proxySettings: ProxySettings = getDefaultProxySettings()!;

it("should return settings with passed port", () => {
const port = 3030;
const proxyUrl = "prot://proxy.microsoft.com";
const proxyUrlWithPort = `${proxyUrl}:${port}`;
const proxySettings: ProxySettings = getDefaultProxySettings(proxyUrlWithPort)!;
proxySettings.host.should.equal(proxyUrl);
proxySettings.port.should.equal(defaultPort);
proxySettings.port.should.equal(port);
});

describe("should prefer HTTPS proxy over HTTP proxy", () => {
[
{ name: "lower case", func: (envVar: string) => envVar.toLowerCase() },
{ name: "upper case", func: (envVar: string) => envVar.toUpperCase() }
].forEach(testCase => {
it(`with ${testCase.name}`, () => {
describe("with loadEnvironmentProxyValue", () => {
beforeEach(() => {
delete process.env[Constants.HTTP_PROXY];
delete process.env[Constants.HTTPS_PROXY];
delete process.env[Constants.HTTP_PROXY.toLowerCase()];
delete process.env[Constants.HTTPS_PROXY.toLowerCase()];
});

it("should return undefined when no proxy passed and environment variable is not set", () => {
const proxySettings: ProxySettings | undefined = getDefaultProxySettings();
should().not.exist(proxySettings);
});

it("should load settings from environment variables when no proxyUrl passed", () => {
const proxyUrl = "http://proxy.azure.com";
process.env[Constants.HTTP_PROXY] = proxyUrl;
const proxySettings: ProxySettings = getDefaultProxySettings()!;

proxySettings.host.should.equal(proxyUrl);
proxySettings.port.should.equal(defaultPort);
});

describe("should prefer HTTPS proxy over HTTP proxy", () => {
[
{ name: "lower case", func: (envVar: string) => envVar.toLowerCase() },
{ name: "upper case", func: (envVar: string) => envVar.toUpperCase() }
].forEach(testCase => {
it(`with ${testCase.name}`, () => {
const httpProxy = "http://proxy.microsoft.com";
const httpsProxy = "https://proxy.azure.com";
process.env[testCase.func(Constants.HTTP_PROXY)] = httpProxy;
process.env[testCase.func(Constants.HTTPS_PROXY)] = httpsProxy;

const proxySettings: ProxySettings = getDefaultProxySettings()!;
proxySettings.host.should.equal(httpsProxy);
proxySettings.port.should.equal(defaultPort);
});
});

it("should prefer HTTPS proxy over HTTP proxy", () => {
const httpProxy = "http://proxy.microsoft.com";
const httpsProxy = "https://proxy.azure.com";
process.env[testCase.func(Constants.HTTP_PROXY)] = httpProxy;
process.env[testCase.func(Constants.HTTPS_PROXY)] = httpsProxy;
process.env[Constants.HTTP_PROXY] = httpProxy;
process.env[Constants.HTTPS_PROXY] = httpsProxy;

const proxySettings: ProxySettings = getDefaultProxySettings()!;
proxySettings.host.should.equal(httpsProxy);
proxySettings.port.should.equal(defaultPort);
});
});

it("should prefer HTTPS proxy over HTTP proxy", () => {
const httpProxy = "http://proxy.microsoft.com";
const httpsProxy = "https://proxy.azure.com";
process.env[Constants.HTTP_PROXY] = httpProxy;
process.env[Constants.HTTPS_PROXY] = httpsProxy;
["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].forEach(envVariableName => {
it(`should should load setting from "${envVariableName}" environmental variable`, () => {
process.env[envVariableName] = proxyUrl;
const proxySettings: ProxySettings = getDefaultProxySettings()!;

const proxySettings: ProxySettings = getDefaultProxySettings()!;
proxySettings.host.should.equal(httpsProxy);
proxySettings.port.should.equal(defaultPort);
proxySettings.host.should.equal(proxyUrl);
proxySettings.port.should.equal(defaultPort);
});
});
});
});

["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].forEach(envVariableName => {
it(`should should load setting from "${envVariableName}" environmental variable`, () => {
process.env[envVariableName] = proxyUrl;
const proxySettings: ProxySettings = getDefaultProxySettings()!;

proxySettings.host.should.equal(proxyUrl);
proxySettings.port.should.equal(defaultPort);
browserDescribe("for browser", () => {
[undefined, "http://proxy.microsoft.com", "https://proxy.azure.com:8080"].forEach(proxyUrl => {
it(`should return undefined for ${proxyUrl}`, () => {
const proxySettings = getDefaultProxySettings(proxyUrl);
should().not.exist(proxySettings);
});
});
});
Expand Down
3 changes: 2 additions & 1 deletion webpack.testconfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const config: webpack.Configuration = {
new webpack.NormalModuleReplacementPlugin(/(\.).+util\/base64/, path.resolve(__dirname, "./lib/util/base64.browser.ts")),
new webpack.NormalModuleReplacementPlugin(/(\.).+util\/xml/, path.resolve(__dirname, "./lib/util/xml.browser.ts")),
new webpack.NormalModuleReplacementPlugin(/(\.).+defaultHttpClient/, path.resolve(__dirname, "./lib/defaultHttpClient.browser.ts")),
new webpack.NormalModuleReplacementPlugin(/(\.).+msRestUserAgentPolicy/, path.resolve(__dirname, "./lib/policies/msRestUserAgentPolicy.browser.ts"))
new webpack.NormalModuleReplacementPlugin(/(\.).+msRestUserAgentPolicy/, path.resolve(__dirname, "./lib/policies/msRestUserAgentPolicy.browser.ts")),
new webpack.NormalModuleReplacementPlugin(/(\.).+proxyPolicy/, path.resolve(__dirname, "./lib/policies/proxyPolicy.browser.ts"))
],
module: {
rules: [
Expand Down

0 comments on commit fafa261

Please sign in to comment.