http: ServerRequest does not timeout after 'end'

Fixes #4967

3 files changed, 86 insertions, 50 deletions

diff --git a/lib/http.js b/lib/http.js
index 2f4bcc51e..0f7ea853c 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -1873,7 +1873,7 @@ function connectionListener(socket) {
     socket.setTimeout(self.timeout);
   socket.on('timeout', function() {
     var req = socket.parser && socket.parser.incoming;
-    var reqTimeout = req && req.emit('timeout', socket);
+    var reqTimeout = req && !req.complete && req.emit('timeout', socket);
     var res = socket._httpMessage;
     var resTimeout = res && res.emit('timeout', socket);
     var serverTimeout = self.emit('timeout', socket);
diff --git a/test/simple/test-http-server-timeout-pipeline.js b/test/simple/test-http-server-timeout-pipeline.js
deleted file mode 100644
index 0e4d41e6f..000000000
--- a/test/simple/test-http-server-timeout-pipeline.js
+++ /dev/null
@@ -1,48 +0,0 @@
-// 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 = http.createServer(function(req, res) {
-  console.log('get', req.url);
-  res.setTimeout(100, function() {
-    console.log('timeout on response', req.url);
-  });
-  req.on('end', function() {
-    console.log('end', req.url);
-  });
-}).listen(3000, function() {
-  var c = net.connect(3000, function() {
-    c.write('GET /1 HTTP/1.1\r\n' +
-      'Host: localhost\r\n\r\n');
-    c.write('GET /2 HTTP/1.1\r\n' +
-      'Host: localhost\r\n\r\n');
-  });
-});
-server.setTimeout(200, function(socket) {
-  console.log('timeout on server');
-  socket.destroy();
-  server.close();
-});
diff --git a/test/simple/test-http-set-timeout-server.js b/test/simple/test-http-set-timeout-server.js
index 21be505b7..ff4144963 100644
--- a/test/simple/test-http-set-timeout-server.js
+++ b/test/simple/test-http-set-timeout-server.js
@@ -22,6 +22,7 @@
 var common = require('../common.js');
 var assert = require('assert');
 var http = require('http');
+var net = require('net');
 
 var tests = [];
 
@@ -73,7 +74,10 @@ test(function serverRequestTimeout(cb) {
     });
   });
   server.listen(common.PORT);
-  http.get({ port: common.PORT }).on('error', function() {});
+  var req = http.request({ port: common.PORT, method: 'POST' });
+  req.on('error', function() {});
+  req.write('Hello');
+  // req is in progress
 });
 
 test(function serverResponseTimeout(cb) {
@@ -93,3 +97,83 @@ test(function serverResponseTimeout(cb) {
   server.listen(common.PORT);
   http.get({ port: common.PORT }).on('error', function() {});
 });
+
+test(function serverRequestNotTimeoutAfterEnd(cb) {
+  var caughtTimeoutOnRequest = false;
+  var caughtTimeoutOnResponse = false;
+  process.on('exit', function() {
+    assert(!caughtTimeoutOnRequest);
+    assert(caughtTimeoutOnResponse);
+  });
+  var server = http.createServer(function(req, res) {
+    // just do nothing, we should get a timeout event.
+    req.setTimeout(50, function(socket) {
+      caughtTimeoutOnRequest = true;
+    });
+    res.on('timeout', function(socket) {
+      caughtTimeoutOnResponse = true;
+    });
+  });
+  server.on('timeout', function(socket) {
+    socket.destroy();
+    server.close();
+    cb();
+  });
+  server.listen(common.PORT);
+  http.get({ port: common.PORT }).on('error', function() {});
+});
+
+test(function serverResponseTimeoutWithPipeline(cb) {
+  var caughtTimeout = '';
+  process.on('exit', function() {
+    assert.equal(caughtTimeout, '/2');
+  });
+  var server = http.createServer(function(req, res) {
+    res.setTimeout(50, function() {
+      caughtTimeout += req.url;
+    });
+    if (req.url === '/1') res.end();
+  });
+  server.on('timeout', function(socket) {
+    socket.destroy();
+    server.close();
+    cb();
+  });
+  server.listen(common.PORT);
+  var c = net.connect({ port: common.PORT, allowHalfOpen: true }, function() {
+    c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
+    c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n');
+    c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n');
+  });
+});
+
+test(function idleTimeout(cb) {
+  var caughtTimeoutOnRequest = false;
+  var caughtTimeoutOnResponse = false;
+  var caughtTimeoutOnServer = false;
+  process.on('exit', function() {
+    assert(!caughtTimeoutOnRequest);
+    assert(!caughtTimeoutOnResponse);
+    assert(caughtTimeoutOnServer);
+  });
+  var server = http.createServer(function(req, res) {
+    req.on('timeout', function(socket) {
+      caughtTimeoutOnRequest = true;
+    });
+    res.on('timeout', function(socket) {
+      caughtTimeoutOnResponse = true;
+    });
+    res.end();
+  });
+  server.setTimeout(50, function(socket) {
+    caughtTimeoutOnServer = true;
+    socket.destroy();
+    server.close();
+    cb();
+  });
+  server.listen(common.PORT);
+  var c = net.connect({ port: common.PORT, allowHalfOpen: true }, function() {
+    c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
+    // Keep-Alive
+  });
+});