Skip to content

Commit a29b5e8

Browse files
committed
Correct keep-alive responses to HTTP 1.0 clients.
Since the proxy requests comes from NodeJS's HTTP 1.1 request client, a backend server may default to setting Connection: keep-alive in its response. However, the real HTTP 1.0 client may not be able to handle that. Force HTTP 1.0 client's to Connection: close, unless the client explicitly supports keep-alive.
1 parent 9c13ad4 commit a29b5e8

File tree

3 files changed

+98
-9
lines changed

3 files changed

+98
-9
lines changed

lib/node-http-proxy/http-proxy.js

+8-7
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,16 @@ HttpProxy.prototype.proxyRequest = function (req, res, buffer) {
248248
//
249249
// Process the `reverseProxy` `response` when it's received.
250250
//
251-
if (!response.headers.connection) {
251+
if (req.httpVersion === '1.0') {
252+
if (req.headers.connection) {
253+
response.headers.connection = req.headers.connection
254+
} else {
255+
response.headers.connection = 'close'
256+
}
257+
} else if (!response.headers.connection) {
252258
if (req.headers.connection) { response.headers.connection = req.headers.connection }
253259
else {
254-
if (req.httpVersion === '1.0') {
255-
response.headers.connection = 'close'
256-
}
257-
else if (req.httpVersion === '1.1') {
258-
response.headers.connection = 'keep-alive'
259-
}
260+
response.headers.connection = 'keep-alive'
260261
}
261262
}
262263

test/http/http-test.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ vows.describe(helpers.describe()).addBatch({
6060
targetHeaders: { connection: "keep-alive" },
6161
outputHeaders: { connection: "keep-alive" }
6262
}),
63+
"and response keep-alive connection header from http 1.0 client": macros.http.assertRawHttpProxied({
64+
rawRequest: "GET / HTTP/1.0\r\n\r\n",
65+
targetHeaders: { connection: "keep-alive" },
66+
match: /connection: close/i
67+
}),
68+
"and request keep alive from http 1.0 client": macros.http.assertRawHttpProxied({
69+
rawRequest: "GET / HTTP/1.0\r\nConnection: Keep-Alive\r\n\r\n",
70+
targetHeaders: { connection: "keep-alive" },
71+
match: /connection: keep-alive/i
72+
}),
6373
"and no connection header": macros.http.assertProxied({
6474
request: { headers: { connection: "" } }, // Must explicitly set to "" because otherwise node will automatically add a "connection: keep-alive" header
6575
outputHeaders: { connection: "keep-alive" }
@@ -89,4 +99,4 @@ vows.describe(helpers.describe()).addBatch({
8999
latency: 2000
90100
})
91101
}
92-
}).export(module);
102+
}).export(module);

test/macros/http.js

+79-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
var assert = require('assert'),
1010
fs = require('fs'),
1111
async = require('async'),
12+
net = require('net'),
1213
request = require('request'),
1314
helpers = require('../helpers/index');
1415

@@ -141,6 +142,83 @@ exports.assertProxied = function (options) {
141142
};
142143
};
143144

145+
//
146+
// ### function assertRawHttpProxied (options)
147+
// #### @options {Object} Options for this test
148+
// #### @rawRequest {string} Raw HTTP request to perform.
149+
// #### @match {RegExp} Output to match in the response.
150+
// #### @latency {number} Latency in milliseconds for the proxy server.
151+
// #### @ports {Object} Ports for the request (target, proxy).
152+
// #### @output {string} Output to assert from.
153+
// #### @forward {Object} Options for forward proxying.
154+
//
155+
// Creates a complete end-to-end test for requesting against an
156+
// http proxy.
157+
//
158+
exports.assertRawHttpProxied = function (options) {
159+
options = options || {};
160+
161+
var ports = options.ports || helpers.nextPortPair,
162+
output = options.output || 'hello world from ' + ports.target,
163+
outputHeaders = options.outputHeaders,
164+
targetHeaders = options.targetHeaders,
165+
proxyHeaders = options.proxyHeaders,
166+
protocol = helpers.protocols.proxy,
167+
timeout = options.timeout || null,
168+
assertFn = options.shouldFail
169+
? exports.assertFailedRequest
170+
: exports.assertRequest;
171+
172+
return {
173+
topic: function () {
174+
var topicCallback = this.callback;
175+
176+
//
177+
// Create a target server and a proxy server
178+
// using the `options` supplied.
179+
//
180+
helpers.http.createServerPair({
181+
target: {
182+
output: output,
183+
outputHeaders: targetHeaders,
184+
port: ports.target,
185+
latency: options.requestLatency
186+
},
187+
proxy: {
188+
latency: options.latency,
189+
port: ports.proxy,
190+
outputHeaders: proxyHeaders,
191+
proxy: {
192+
forward: options.forward,
193+
target: {
194+
https: helpers.protocols.target === 'https',
195+
host: '127.0.0.1',
196+
port: ports.target
197+
},
198+
timeout: timeout
199+
}
200+
}
201+
}, function() {
202+
var response = '';
203+
var client = net.connect(ports.proxy, '127.0.0.1', function() {
204+
client.write(options.rawRequest);
205+
});
206+
207+
client.on('data', function(data) {
208+
response += data.toString();
209+
});
210+
211+
client.on('end', function() {
212+
topicCallback(null, options.match, response);
213+
});
214+
});
215+
},
216+
"should succeed": function(err, match, response) {
217+
assert.match(response, match);
218+
}
219+
};
220+
};
221+
144222
//
145223
// ### function assertInvalidProxy (options)
146224
// #### @options {Object} Options for this test
@@ -444,4 +522,4 @@ exports.assertDynamicProxy = function (static, dynamic) {
444522
return exports.assertProxiedToRoutes(static, {
445523
"once the server has started": context
446524
});
447-
};
525+
};

0 commit comments

Comments
 (0)