Merge pull request #407 from GUI/master

Correct keep-alive responses to HTTP 1.0 clients

Author: indexzero

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'
       }
     }
 

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);
+}).export(module);

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