http: Don't try to destroy nonexistent sockets

Fixes #3740

In the case of pipelined requests, you can have a situation where
the socket gets destroyed via one req/res object, but then trying
to destroy *another* req/res on the same socket will cause it to
call undefined.destroy(), since it was already removed from that
message.

Add a guard to OutgoingMessage.destroy and IncomingMessage.destroy
to prevent this error.

diff --git a/lib/http.js b/lib/http.js
index ac6b1c6bf7ae7a49d2926471b2461a78515ad8c4..4a4fe94f3ec63095da48d6371dc373e30bca181f 100644 (file)
--- a/lib/http.js
+++ b/lib/http.js
@@ -349,8 +349,12 @@ IncomingMessage.prototype._read = function(n) {
 };
 
 
+// It's possible that the socket will be destroyed, and removed from
+// any messages, before ever calling this.  In that case, just skip
+// it, since something else is destroying this connection anyway.
 IncomingMessage.prototype.destroy = function(error) {
-  this.socket.destroy(error);
+  if (this.socket)
+    this.socket.destroy(error);
 };
 
 
@@ -467,8 +471,16 @@ OutgoingMessage.prototype.setTimeout = function(msecs, callback) {
 };
 
 
+// It's possible that the socket will be destroyed, and removed from
+// any messages, before ever calling this.  In that case, just skip
+// it, since something else is destroying this connection anyway.
 OutgoingMessage.prototype.destroy = function(error) {
-  this.socket.destroy(error);
+  if (this.socket)
+    this.socket.destroy(error);
+  else
+    this.once('socket', function(socket) {
+      socket.destroy(error);
+    });
 };
 
 

diff --git a/test/simple/test-http-incoming-pipelined-socket-destroy.js b/test/simple/test-http-incoming-pipelined-socket-destroy.js
new file mode 100644 (file)
index 0000000..679ab46
--- /dev/null
+++ b/test/simple/test-http-incoming-pipelined-socket-destroy.js
@@ -0,0 +1,81 @@
+// 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');
+
+
+// Set up some timing issues where sockets can be destroyed
+// via either the req or res.
+var server = http.createServer(function(req, res) {
+  switch (req.url) {
+    case '/1':
+      return setTimeout(function() {
+        req.socket.destroy();
+      });
+
+    case '/2':
+      return process.nextTick(function() {
+        res.destroy();
+      });
+
+    // in one case, actually send a response in 2 chunks
+    case '/3':
+      res.write('hello ');
+      return setTimeout(function() {
+        res.end('world!');
+      });
+
+    default:
+      return res.destroy();
+  }
+});
+
+
+// Make a bunch of requests pipelined on the same socket
+function generator() {
+  var reqs = [ 3, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4 ];
+  return reqs.map(function(r) {
+    return 'GET /' + r + ' HTTP/1.1\r\n' +
+           'Host: localhost:' + common.PORT + '\r\n' +
+           '\r\n' +
+           '\r\n'
+  }).join('');
+}
+
+
+server.listen(common.PORT, function() {
+  var client = net.connect({ port: common.PORT });
+
+  // immediately write the pipelined requests.
+  // Some of these will not have a socket to destroy!
+  client.write(generator(), function() {
+    server.close();
+  });
+});
+
+process.on('exit', function(c) {
+  if (!c)
+    console.log('ok');
+});