http: keep-alive should default with HTTP/1.1 server

As RFC 2616 says we should, assume that servers will provide a persistent
connection by default.

> A significant difference between HTTP/1.1 and earlier versions of
> HTTP is that persistent connections are the default behavior of any
> HTTP connection. That is, unless otherwise indicated, the client
> SHOULD assume that the server will maintain a persistent connection,
> even after error responses from the server.

> HTTP/1.1 applications that do not support persistent connections MUST
> include the "close" connection option in every message.

Fixes #2436.

Author: koichik

lib/http.js

@@ -1231,7 +1231,7 @@ ClientRequest.prototype.onSocket = function(socket) {
         return true;
       }
 
-      if (req.shouldKeepAlive && res.headers.connection !== 'keep-alive' && !req.upgraded) {
+      if (req.shouldKeepAlive && !shouldKeepAlive && !req.upgraded) {
         // Server MUST respond with Connection:keep-alive for us to enable it.
         // If we've been upgraded (via WebSockets) we also shouldn't try to
         // keep the connection open.

test/simple/test-http-should-keep-alive.js

@@ -0,0 +1,71 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var http = require('http');
+var net = require('net');
+
+var SERVER_RESPONSES = [
+  'HTTP/1.0 200 ok\r\nContent-Length: 0\r\n\r\n',
+  'HTTP/1.0 200 ok\r\nContent-Length: 0\r\nConnection: keep-alive\r\n\r\n',
+  'HTTP/1.0 200 ok\r\nContent-Length: 0\r\nConnection: close\r\n\r\n',
+  'HTTP/1.1 200 ok\r\nContent-Length: 0\r\n\r\n',
+  'HTTP/1.1 200 ok\r\nContent-Length: 0\r\nConnection: keep-alive\r\n\r\n',
+  'HTTP/1.1 200 ok\r\nContent-Length: 0\r\nConnection: close\r\n\r\n',
+];
+var SHOULD_KEEP_ALIVE = [
+  false, // HTTP/1.0, default
+  true,  // HTTP/1.0, Connection: keep-alive
+  false, // HTTP/1.0, Connection: close
+  true,  // HTTP/1.1, default
+  true,  // HTTP/1.1, Connection: keep-alive
+  false, // HTTP/1.1, Connection: close
+];
+var requests = 0;
+var responses = 0;
+
+var server = net.createServer(function(socket) {
+  socket.write(SERVER_RESPONSES[requests]);
+  ++requests;
+}).listen(common.PORT, function() {
+  function makeRequest() {
+    var req = http.get({port: common.PORT}, function(res) {
+      assert.equal(req.shouldKeepAlive, SHOULD_KEEP_ALIVE[responses],
+                   SERVER_RESPONSES[responses] + ' should ' +
+                   (SHOULD_KEEP_ALIVE[responses] ? '' : 'not ') +
+                   'Keep-Alive');
+      ++responses;
+      if (responses < SHOULD_KEEP_ALIVE.length) {
+        makeRequest();
+      } else {
+        server.close();
+      }
+    });
+  }
+
+  makeRequest();
+});
+
+process.on('exit', function() {
+  assert.equal(requests, SERVER_RESPONSES.length);
+  assert.equal(responses, SHOULD_KEEP_ALIVE.length);
+});