http: handle aborts

5 files changed, 137 insertions, 3 deletions

diff --git a/doc/api/http.markdown b/doc/api/http.markdown
index b21314b038e..3e75bf36c03 100644
--- a/doc/api/http.markdown
+++ b/doc/api/http.markdown
@@ -503,6 +503,10 @@ chunked, this will send the terminating `'0\r\n\r\n'`.
 If `data` is specified, it is equivalent to calling `request.write(data, encoding)`
 followed by `request.end()`.
 
+### request.abort()
+
+Aborts a request.  (New since v0.3.80.)
+
 
 ## http.ClientResponse
 
diff --git a/lib/http.js b/lib/http.js
index 3b38aa1ee23..efeb9d24159 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -763,6 +763,25 @@ util.inherits(ClientRequest, OutgoingMessage);
 exports.ClientRequest = ClientRequest;
 
 
+ClientRequest.prototype.abort = function() {
+  if (this._queue) {
+    // queued for dispatch
+    assert(!this.connection);
+    var i = this._queue.indexOf(this);
+    this._queue.splice(i, 1);
+
+  } else if (this.connection) {
+    // in-progress
+    var c = this.connection;
+    this.detachSocket(c);
+    c.destroy();
+
+  } else {
+    // already complete.
+  }
+};
+
+
 function httpSocketSetup(socket) {
   // NOTE: be sure not to use ondrain elsewhere in this file!
   socket.ondrain = function() {
@@ -781,6 +800,11 @@ function Server(requestListener) {
     this.addListener('request', requestListener);
   }
 
+  // Similar option to this. Too lazy to write my own docs.
+  // http://www.squid-cache.org/Doc/config/half_closed_clients/
+  // http://wiki.squid-cache.org/SquidFaq/InnerWorkings#What_is_a_half-closed_filedescriptor.3F
+  this.httpAllowHalfOpen = false;
+
   this.addListener('connection', connectionListener);
 }
 util.inherits(Server, net.Server);
@@ -797,6 +821,15 @@ exports.createServer = function(requestListener) {
 function connectionListener(socket) {
   var self = this;
   var outgoing = [];
+  var incoming = [];
+
+  function abortIncoming() {
+    while (incoming.length) {
+      var req = incoming.shift();
+      req.emit('aborted');
+    }
+    // abort socket._httpMessage ?
+  }
 
   debug('SERVER new http connection');
 
@@ -842,9 +875,18 @@ function connectionListener(socket) {
   };
 
   socket.onend = function() {
-    parser.finish();
+    var ret = parser.finish();
 
-    if (outgoing.length) {
+    if (ret instanceof Error) {
+      debug('parse error');
+      socket.destroy(ret);
+      return;
+    }
+
+    if (!self.httpAllowHalfOpen) {
+      abortIncoming();
+      socket.end();
+    } else if (outgoing.length) {
       outgoing[outgoing.length - 1]._last = true;
     } else if (socket._httpMessage) {
       socket._httpMessage._last = true;
@@ -854,14 +896,19 @@ function connectionListener(socket) {
   };
 
   socket.addListener('close', function() {
+    debug('server socket close');
     // unref the parser for easy gc
     parsers.free(parser);
+
+    abortIncoming();
   });
 
   // The following callback is issued after the headers have been read on a
   // new message. In this callback we setup the response object and pass it
   // to the user.
   parser.onIncoming = function(req, shouldKeepAlive) {
+    incoming.push(req);
+
     var res = new ServerResponse(req);
     debug('server response shouldKeepAlive: ' + shouldKeepAlive);
     res.shouldKeepAlive = shouldKeepAlive;
@@ -877,6 +924,9 @@ function connectionListener(socket) {
     // When we're finished writing the response, check if this is the last
     // respose, if so destroy the socket.
     res.on('finish', function() {
+      assert(incoming[0] === req);
+      incoming.shift();
+
       res.detachSocket(socket);
 
       if (res._last) {
@@ -909,23 +959,28 @@ function connectionListener(socket) {
 exports._connectionListener = connectionListener;
 
 
+
 function Agent(host, port) {
   this.host = host;
   this.port = port;
 
   this.queue = [];
   this.sockets = [];
-  this.maxSockets = 5;
+  this.maxSockets = Agent.defaultMaxSockets;
 }
 util.inherits(Agent, EventEmitter);
 exports.Agent = Agent;
 
 
+Agent.defaultMaxSockets = 5;
+
+
 Agent.prototype.appendMessage = function(options) {
   var self = this;
 
   var req = new ClientRequest(options);
   this.queue.push(req);
+  req._queue = this.queue;
 
   /*
   req.on('finish', function () {
@@ -978,6 +1033,8 @@ Agent.prototype._establishNewConnection = function() {
       req = socket._httpMessage;
     } else if (self.queue.length) {
       req = self.queue.shift();
+      assert(req._queue === self.queue);
+      req._queue = null;
     } else {
       // No requests on queue? Where is the request
       assert(0);
@@ -1135,6 +1192,9 @@ Agent.prototype._cycle = function() {
       debug('Agent found socket, shift');
       // We found an available connection!
       this.queue.shift(); // remove first from queue.
+      assert(first._queue === this.queue);
+      first._queue = null;
+
       first.assignSocket(socket);
       self._cycle(); // try to dispatch another
       return;
diff --git a/lib/net.js b/lib/net.js
index 8c387df0e49..965d485207a 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -754,6 +754,8 @@ Socket.prototype.destroy = function(exception) {
   // pool is shared between sockets, so don't need to free it here.
   var self = this;
 
+  debug('destroy ' + this.fd);
+
   // TODO would like to set _writeQueue to null to avoid extra object alloc,
   // but lots of code assumes this._writeQueue is always an array.
   assert(this.bufferSize >= 0);
@@ -787,6 +789,7 @@ Socket.prototype.destroy = function(exception) {
 
   // FIXME Bug when this.fd == 0
   if (typeof this.fd == 'number') {
+    debug('close ' + this.fd);
     close(this.fd);
     this.fd = null;
     process.nextTick(function() {
diff --git a/test/simple/test-http-client-abort.js b/test/simple/test-http-client-abort.js
new file mode 100644
index 00000000000..3e0199b3272
--- /dev/null
+++ b/test/simple/test-http-client-abort.js
@@ -0,0 +1,59 @@
+var common = require('../common');
+var assert = require('assert');
+var http = require('http');
+
+var clientAborts = 0;
+
+var server = http.Server(function(req, res) {
+  console.log("Got connection");
+  res.writeHead(200);
+  res.write("Working on it...");
+
+  // I would expect an error event from req or res that the client aborted
+  // before completing the HTTP request / response cycle, or maybe a new
+  // event like "aborted" or something.
+  req.on("aborted", function () {
+    clientAborts++;
+    console.log("Got abort " + clientAborts);
+    if (clientAborts === N) {
+      console.log("All aborts detected, you win.");
+      server.close();
+    }
+  });
+
+  // since there is already clientError, maybe that would be appropriate,
+  // since "error" is magical
+  req.on("clientError", function () {
+    console.log("Got clientError");
+  });
+});
+
+var responses = 0;
+var N = http.Agent.defaultMaxSockets - 1;
+var requests = [];
+
+server.listen(common.PORT, function() {
+  console.log("Server listening.");
+
+  for (var i = 0; i < N; i++) {
+    console.log("Making client " + i);
+    var options = { port: common.PORT, path: '/?id=' + i };
+    var req = http.get(options, function(res) {
+      console.log("Client response code " + res.statusCode);
+
+      if (++responses == N) {
+        console.log("All clients connected, destroying.");
+        requests.forEach(function (outReq) {
+          console.log("abort");
+          outReq.abort();
+        });
+      }
+    });
+
+    requests.push(req);
+  }
+});
+
+process.on('exit', function() {
+  assert.equal(N, clientAborts);
+});
diff --git a/test/simple/test-http-server.js b/test/simple/test-http-server.js
index 969b6d35d6c..3784439f0b4 100644
--- a/test/simple/test-http-server.js
+++ b/test/simple/test-http-server.js
@@ -48,6 +48,8 @@ var server = http.createServer(function(req, res) {
 });
 server.listen(common.PORT);
 
+server.httpAllowHalfOpen = true;
+
 server.addListener('listening', function() {
   var c = net.createConnection(common.PORT);
 
@@ -69,6 +71,12 @@ server.addListener('listening', function() {
     if (requests_sent == 2) {
       c.write('GET / HTTP/1.1\r\nX-X: foo\r\n\r\n' +
               'GET / HTTP/1.1\r\nX-X: bar\r\n\r\n');
+      // Note: we are making the connection half-closed here
+      // before we've gotten the response from the server. This
+      // is a pretty bad thing to do and not really supported
+      // by many http servers. Node supports it optionally if
+      // you set server.httpAllowHalfOpen=true, which we've done
+      // above.
       c.end();
       assert.equal(c.readyState, 'readOnly');
       requests_sent += 2;