Skip to content

Commit

Permalink
Correct keep-alive responses to HTTP 1.0 clients.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
GUI committed Apr 18, 2013
1 parent 9c13ad4 commit a29b5e8
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 9 deletions.
15 changes: 8 additions & 7 deletions lib/node-http-proxy/http-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,16 @@ HttpProxy.prototype.proxyRequest = function (req, res, buffer) {
//
// Process the `reverseProxy` `response` when it's received.
//
if (!response.headers.connection) {
if (req.httpVersion === '1.0') {
if (req.headers.connection) {
response.headers.connection = req.headers.connection
} else {
response.headers.connection = 'close'
}
} else if (!response.headers.connection) {
if (req.headers.connection) { response.headers.connection = req.headers.connection }
else {
if (req.httpVersion === '1.0') {
response.headers.connection = 'close'
}
else if (req.httpVersion === '1.1') {
response.headers.connection = 'keep-alive'
}
response.headers.connection = 'keep-alive'
}
}

Expand Down
12 changes: 11 additions & 1 deletion test/http/http-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ vows.describe(helpers.describe()).addBatch({
targetHeaders: { connection: "keep-alive" },
outputHeaders: { connection: "keep-alive" }
}),
"and response keep-alive connection header from http 1.0 client": macros.http.assertRawHttpProxied({
rawRequest: "GET / HTTP/1.0\r\n\r\n",
targetHeaders: { connection: "keep-alive" },
match: /connection: close/i
}),
"and request keep alive from http 1.0 client": macros.http.assertRawHttpProxied({
rawRequest: "GET / HTTP/1.0\r\nConnection: Keep-Alive\r\n\r\n",
targetHeaders: { connection: "keep-alive" },
match: /connection: keep-alive/i
}),
"and no connection header": macros.http.assertProxied({
request: { headers: { connection: "" } }, // Must explicitly set to "" because otherwise node will automatically add a "connection: keep-alive" header
outputHeaders: { connection: "keep-alive" }
Expand Down Expand Up @@ -89,4 +99,4 @@ vows.describe(helpers.describe()).addBatch({
latency: 2000
})
}
}).export(module);
}).export(module);
80 changes: 79 additions & 1 deletion test/macros/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
var assert = require('assert'),
fs = require('fs'),
async = require('async'),
net = require('net'),
request = require('request'),
helpers = require('../helpers/index');

Expand Down Expand Up @@ -141,6 +142,83 @@ exports.assertProxied = function (options) {
};
};

//
// ### function assertRawHttpProxied (options)
// #### @options {Object} Options for this test
// #### @rawRequest {string} Raw HTTP request to perform.
// #### @match {RegExp} Output to match in the response.
// #### @latency {number} Latency in milliseconds for the proxy server.
// #### @ports {Object} Ports for the request (target, proxy).
// #### @output {string} Output to assert from.
// #### @forward {Object} Options for forward proxying.
//
// Creates a complete end-to-end test for requesting against an
// http proxy.
//
exports.assertRawHttpProxied = function (options) {
options = options || {};

var ports = options.ports || helpers.nextPortPair,
output = options.output || 'hello world from ' + ports.target,
outputHeaders = options.outputHeaders,
targetHeaders = options.targetHeaders,
proxyHeaders = options.proxyHeaders,
protocol = helpers.protocols.proxy,
timeout = options.timeout || null,
assertFn = options.shouldFail
? exports.assertFailedRequest
: exports.assertRequest;

return {
topic: function () {
var topicCallback = this.callback;

//
// Create a target server and a proxy server
// using the `options` supplied.
//
helpers.http.createServerPair({
target: {
output: output,
outputHeaders: targetHeaders,
port: ports.target,
latency: options.requestLatency
},
proxy: {
latency: options.latency,
port: ports.proxy,
outputHeaders: proxyHeaders,
proxy: {
forward: options.forward,
target: {
https: helpers.protocols.target === 'https',
host: '127.0.0.1',
port: ports.target
},
timeout: timeout
}
}
}, function() {
var response = '';
var client = net.connect(ports.proxy, '127.0.0.1', function() {
client.write(options.rawRequest);
});

client.on('data', function(data) {
response += data.toString();
});

client.on('end', function() {
topicCallback(null, options.match, response);
});
});
},
"should succeed": function(err, match, response) {
assert.match(response, match);
}
};
};

//
// ### function assertInvalidProxy (options)
// #### @options {Object} Options for this test
Expand Down Expand Up @@ -444,4 +522,4 @@ exports.assertDynamicProxy = function (static, dynamic) {
return exports.assertProxiedToRoutes(static, {
"once the server has started": context
});
};
};

0 comments on commit a29b5e8

Please sign in to comment.