Skip to content

Correct keep-alive responses to HTTP 1.0 clients #407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 21, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
86 changes: 85 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,89 @@ 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) {
// Don't test raw requests over HTTPS since options.rawRequest won't be
// encrypted.
if(helpers.protocols.proxy == 'https') {
return true;
}

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 +528,4 @@ exports.assertDynamicProxy = function (static, dynamic) {
return exports.assertProxiedToRoutes(static, {
"once the server has started": context
});
};
};