Skip to content

Commit

Permalink
disables automatic proxy detection, fixes #672
Browse files Browse the repository at this point in the history
  • Loading branch information
aoberoi committed Mar 7, 2019
1 parent c640509 commit 4b81bfd
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 3 deletions.
5 changes: 4 additions & 1 deletion package.json
Expand Up @@ -35,7 +35,7 @@
"lint": "tslint --project .",
"test": "npm run build && npm run test:spec && npm run test:integration",
"test:spec": "nyc mocha --opts src/mocha.opts src/*.spec.js src/**/*.spec.js",
"test:integration": "mocha --opts test/mocha.opts test/typescript/test.ts",
"test:integration": "mocha --opts test/mocha.opts test/typescript/test.ts test/proxy/test.js",
"coverage": "codecov",
"build": "npm run build:clean && tsc",
"build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output",
Expand Down Expand Up @@ -72,13 +72,16 @@
"busboy": "^0.2.14",
"chai": "^4.1.2",
"codecov": "^3.0.0",
"http-shutdown": "^1.2.0",
"https-proxy-agent": "^2.2.1",
"husky": "^0.14.3",
"jsdoc-to-markdown": "^4.0.1",
"lint-staged": "^6.1.0",
"mocha": "^5.0.0",
"nock": "^9.1.6",
"nyc": "^13.3.0",
"p-is-promise": "^1.1.0",
"proxy": "^0.2.4",
"shx": "^0.2.2",
"sinon": "^4.2.2",
"source-map-support": "^0.5.3",
Expand Down
3 changes: 2 additions & 1 deletion src/WebClient.ts
Expand Up @@ -177,6 +177,7 @@ export class WebClient extends EventEmitter {
transformRequest: [this.serializeApiCallOptions.bind(this)],
validateStatus: () => true, // all HTTP status codes should result in a resolved promise (as opposed to only 2xx)
maxRedirects: 0,
proxy: false,
});
// serializeApiCallOptions will always determine the appropriate content-type
delete this.axios.defaults.headers.post['Content-Type'];
Expand Down Expand Up @@ -709,7 +710,7 @@ export class WebClient extends EventEmitter {

return response;
} catch (error) {
this.logger.debug('http request failed');
this.logger.debug('http request failed', error.message);
if (error.request) {
throw requestErrorWithOriginal(error);
}
Expand Down
69 changes: 69 additions & 0 deletions test/proxy/test.js
@@ -0,0 +1,69 @@
require('mocha');
const { createServer } = require('http');
const shutdown = require('http-shutdown');
const setup = require('proxy');
const HttpsProxyAgent = require('https-proxy-agent');
const { assert } = require('chai');
const sinon = require('sinon');
const { WebClient, retryPolicies } = require('../../dist');

const proxyPort = 31280;

describe('with a proxy server listening', function () {
beforeEach(function (done) {
this.proxyUrl = `http://localhost:${proxyPort}`;
this.server = shutdown(setup(createServer()));
this.server.listen(proxyPort, done);
});

it('should connect to a host through the proxy when a proxy agent is configured', function () {
const agent = new HttpsProxyAgent(this.proxyUrl);
const client = new WebClient(undefined, { agent, retryConfig: retryPolicies.rapidRetryPolicy });

// Hooking into authenticate just to observe when an inbound request is seen at the proxy
const spy = sinon.spy();
this.server.authenticate = (req, fn) => {
spy();
fn(null, true);
};

return client.apiCall('method')
.catch(() => {
assert(spy.called);
});
});

it('should NOT connect to a proxy when an environment variable is used', function () {
// manipulate the environment
process.env.http_proxy = this.proxyUrl;
after(function () {
delete process.env.http_proxy;
});

// NOTE: we are intentionally using a non-TLS URL here to make this test effective.
// it turns out that axios' automatic proxy support (which we've disabled) doesn't support TLS connections via
// proxies, while the https-proxy-agent package does. so the only want to test whether the automatic proxy support
// is actually turned off is to use an http (non-TLS) API URL, because otherwise we'd still see the effect that
// the proxy was not called, and that would be a false positive.
const client = new WebClient(undefined, {
slackApiUrl: 'http://example.com/',
retryConfig: retryPolicies.rapidRetryPolicy
});

// Hooking into authenticate just to observe when an inbound request is seen at the proxy
const spy = sinon.spy();
this.server.authenticate = (req, fn) => {
spy();
fn(null, true);
};

return client.apiCall('method')
.catch(() => {
assert(spy.notCalled);
});
});

afterEach(function (done) {
this.server.shutdown(done);
});
});
2 changes: 1 addition & 1 deletion test/typescript/test.ts
@@ -1,5 +1,5 @@
import 'mocha';
import { check, checkDirectory } from 'typings-tester';
import { check } from 'typings-tester';

describe('typescript typings tests', () => {
it('should allow WebClient to work without a token', () => {
Expand Down

0 comments on commit 4b81bfd

Please sign in to comment.