Permalink
Browse files

http: ServerRequest does not timeout after 'end'

Fixes #4967
  • Loading branch information...
1 parent e2400f8 commit c9a4ec9c6392fefa7ce6c66168f4c535ea0702e1 @koichik committed Mar 10, 2013
Showing with 86 additions and 50 deletions.
  1. +1 −1 lib/http.js
  2. +0 −48 test/simple/test-http-server-timeout-pipeline.js
  3. +85 −1 test/simple/test-http-set-timeout-server.js
View
@@ -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);
@@ -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();
-});
@@ -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
+ });
+});

0 comments on commit c9a4ec9

Please sign in to comment.