From a29b5e8e289c34c00d2b450e5fb9dd1969db4b97 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Thu, 18 Apr 2013 16:33:10 -0600 Subject: [PATCH 1/2] 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. --- lib/node-http-proxy/http-proxy.js | 15 +++--- test/http/http-test.js | 12 ++++- test/macros/http.js | 80 ++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/lib/node-http-proxy/http-proxy.js b/lib/node-http-proxy/http-proxy.js index ffd5c957d..9ac8fa6fb 100644 --- a/lib/node-http-proxy/http-proxy.js +++ b/lib/node-http-proxy/http-proxy.js @@ -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' } } diff --git a/test/http/http-test.js b/test/http/http-test.js index b25aa8514..81f8726a7 100644 --- a/test/http/http-test.js +++ b/test/http/http-test.js @@ -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" } @@ -89,4 +99,4 @@ vows.describe(helpers.describe()).addBatch({ latency: 2000 }) } -}).export(module); \ No newline at end of file +}).export(module); diff --git a/test/macros/http.js b/test/macros/http.js index 9f78f8285..89e15f200 100644 --- a/test/macros/http.js +++ b/test/macros/http.js @@ -9,6 +9,7 @@ var assert = require('assert'), fs = require('fs'), async = require('async'), + net = require('net'), request = require('request'), helpers = require('../helpers/index'); @@ -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 @@ -444,4 +522,4 @@ exports.assertDynamicProxy = function (static, dynamic) { return exports.assertProxiedToRoutes(static, { "once the server has started": context }); -}; \ No newline at end of file +}; From daf53bd753879223dc84a49c92d0efaf576c1fd3 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Thu, 18 Apr 2013 18:11:58 -0600 Subject: [PATCH 2/2] Don't test raw HTTP 1.0 requests over HTTPS. --- test/macros/http.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/macros/http.js b/test/macros/http.js index 89e15f200..d3d83426a 100644 --- a/test/macros/http.js +++ b/test/macros/http.js @@ -156,6 +156,12 @@ exports.assertProxied = function (options) { // 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,