Skip to content

Commit

Permalink
fix: invalid CONNECT responses (#151)
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak authored Aug 24, 2021
1 parent cdfbb20 commit 0043ac9
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 5 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "proxy-chain",
"version": "1.0.3",
"version": "1.0.4",
"description": "Node.js implementation of a proxy server (think Squid) with support for SSL, authentication, upstream proxy chaining, and protocol tunneling.",
"main": "build/index.js",
"keywords": [
Expand Down
16 changes: 12 additions & 4 deletions src/handler_tunnel_chain.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ export default class HandlerTunnelChain extends HandlerBase {
run() {
this.log('Connecting to upstream proxy...');

const targetHost = `${this.trgParsed.hostname}:${this.trgParsed.port}`;
this.targetHost = `${this.trgParsed.hostname}:${this.trgParsed.port}`;

const options = {
method: 'CONNECT',
hostname: this.upstreamProxyUrlParsed.hostname,
port: this.upstreamProxyUrlParsed.port,
path: targetHost,
path: this.targetHost,
headers: {
Host: targetHost,
Host: this.targetHost,
},
};

Expand All @@ -44,6 +44,7 @@ export default class HandlerTunnelChain extends HandlerBase {

onTrgRequestConnect(response, socket, head) {
if (this.isClosed) return;

this.log('Connected to upstream proxy');

// Attempt to fix https://github.com/apify/proxy-chain/issues/64,
Expand All @@ -56,7 +57,7 @@ export default class HandlerTunnelChain extends HandlerBase {

this.srcGotResponse = true;
this.srcResponse.removeListener('finish', this.onSrcResponseFinish);
this.srcResponse.writeHead(200, 'Connection Established');
this.srcResponse.writeHead(response.statusCode === 200 ? 200 : 502);

this.emit('tunnelConnectResponded', { response, socket, head });

Expand All @@ -65,6 +66,13 @@ export default class HandlerTunnelChain extends HandlerBase {
// See also https://github.com/nodejs/node/blob/master/lib/_http_outgoing.js#L217
this.srcResponse._send('');

if (response.statusCode !== 200) {
this.log(`Failed to connect to ${this.targetHost} via ${this.upstreamProxyUrlParsed.hostname} (${response.statusCode})`);

this.close();
return;
}

// It can happen that this.close() it called in the meanwhile, so this.srcSocket becomes null
// and the detachSocket() call below fails with "Cannot read property '_httpMessage' of null"
// See https://github.com/apify/proxy-chain/issues/63
Expand Down
59 changes: 59 additions & 0 deletions test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const WebSocket = require('faye-websocket');
const { parseUrl, parseProxyAuthorizationHeader } = require('../build/tools');
const { Server, RequestError } = require('../build/index');
const { TargetServer } = require('./target_server');
const ProxyChain = require('../build/index');

/*
TODO - add following tests:
Expand Down Expand Up @@ -1063,6 +1064,64 @@ describe('Server (HTTPS -> Target)', createTestSuite({
useMainProxy: false,
}));

describe('non-200 upstream connect response', () => {
// No assertion planning :(
// https://github.com/chaijs/chai/issues/670
let success = false;

after(() => {
if (!success) {
throw new Error('Failed');
}
});

it('fails downstream with 502', (done) => {
const server = http.createServer();
server.on('connect', (_request, socket) => {
socket.once('error', () => {});
socket.end('HTTP/1.1 403 Forbidden\r\ncontent-length: 1\r\n\r\na');
});
server.listen(() => {
const serverPort = server.address().port;
const proxyServer = new ProxyChain.Server({
port: 0,
prepareRequestFunction: () => {
return {
upstreamProxyUrl: `http://localhost:${serverPort}`,
};
},
});
proxyServer.listen(() => {
const proxyServerPort = proxyServer.port;

const req = http.request({
method: 'CONNECT',
host: 'localhost',
port: proxyServerPort,
path: 'example.com:443',
headers: {
host: 'example.com:443',
},
});
req.once('connect', (response, socket, head) => {
expect(response.statusCode).to.equal(502);
expect(head.length).to.equal(0);
success = true;

socket.once('close', () => {
proxyServer.close();
server.close();

done();
});
});

req.end();
});
});
});
});

// Run all combinations of test parameters
const useSslVariants = [
false,
Expand Down

0 comments on commit 0043ac9

Please sign in to comment.