From 735b9d50a3cb09c9a87045fce1493036fc742295 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Mon, 29 Nov 2010 14:07:55 -0800
Subject: [PATCH] Simplify state transitions in http.Client

Fixes new bug shown in test-http-allow-req-after-204-res.js pointed out by
Tom Carden <tom.carden@gmail.com>.
---
 lib/http.js                                   | 32 +++++++++-----
 .../test-http-allow-req-after-204-res.js      | 43 +++++++++++++++++++
 2 files changed, 64 insertions(+), 11 deletions(-)
 create mode 100644 test/simple/test-http-allow-req-after-204-res.js

diff --git a/lib/http.js b/lib/http.js
index 883ab088ca..89e2d81a09 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -880,6 +880,12 @@ function Client ( ) {
   net.Stream.call(this, { allowHalfOpen: true });
   var self = this;
 
+  // Possible states:
+  // - disconnected
+  // - connecting
+  // - connected
+  this._state = 'disconnected';
+
   httpSocketSetup(self);
 
   function onData(d, start, end) {
@@ -912,6 +918,8 @@ function Client ( ) {
     self.ondata = onData;
     self.onend = onEnd;
 
+    self._state = "connected";
+
     self._initParser();
     debug('CLIENT requests: ' + util.inspect(self._outgoing.map(function (r) { return r.method; })));
     outgoingFlush(self);
@@ -919,21 +927,22 @@ function Client ( ) {
 
   function onEnd() {
     if (self.parser) self.parser.finish();
-    debug("CLIENT got end closing. readyState = " + self.readyState);
+    debug("CLIENT got end closing. state = " + self._state);
     self.end();
   };
 
   self.addListener("close", function (e) {
+    self._state = "disconnected";
     if (e) return;
 
-    debug("CLIENT onClose. readyState = " + self.readyState);
+    debug("CLIENT onClose. state = " + self._state);
 
     // finally done with the request
     self._outgoing.shift();
 
     // If there are more requests to handle, reconnect.
     if (self._outgoing.length) {
-      self._reconnect();
+      self._ensureConnection();
     } else if (self.parser) {
       parsers.free(self.parser);
       self.parser = null;
@@ -982,7 +991,7 @@ Client.prototype._initParser = function () {
     }
 
     res.addListener('end', function ( ) {
-      debug("CLIENT request complete disconnecting. readyState = " + self.readyState);
+      debug("CLIENT request complete disconnecting. state = " + self._state);
       // For the moment we reconnect for every request. FIXME!
       // All that should be required for keep-alive is to not reconnect,
       // but outgoingFlush instead.
@@ -1020,17 +1029,18 @@ Client.prototype._onOutgoingSent = function (message) {
   //
   // Instead, we just check if the connection is closed, and if so
   // reconnect if we have pending messages.
-  if (this._outgoing.length && this.readyState == "closed") {
-    debug("CLIENT request flush. reconnect.  readyState = " + this.readyState);
-    this._reconnect();
+  if (this._outgoing.length) {
+    debug("CLIENT request flush. ensure connection.  state = " + this._state);
+    this._ensureConnection();
   }
 };
 
 
-Client.prototype._reconnect = function () {
-  if (this.readyState === "closed") {
-    debug("CLIENT reconnecting readyState = " + this.readyState);
+Client.prototype._ensureConnection = function () {
+  if (this._state == 'disconnected') {
+    debug("CLIENT reconnecting state = " + this._state);
     this.connect(this.port, this.host);
+    this._state = "connecting";
   }
 };
 
@@ -1044,7 +1054,7 @@ Client.prototype.request = function (method, url, headers) {
   }
   var req = new ClientRequest(this, method, url, headers);
   this._outgoing.push(req);
-  if (this.readyState === 'closed') this._reconnect();
+  this._ensureConnection();
   return req;
 };
 
diff --git a/test/simple/test-http-allow-req-after-204-res.js b/test/simple/test-http-allow-req-after-204-res.js
new file mode 100644
index 0000000000..6139c6f489
--- /dev/null
+++ b/test/simple/test-http-allow-req-after-204-res.js
@@ -0,0 +1,43 @@
+var common = require('../common');
+var http = require('http');
+var assert = require('assert');
+
+// first 204 or 304 works, subsequent anything fails
+var codes = [ 204, 200 ];
+
+// Methods don't really matter, but we put in something realistic.
+var methods = ['DELETE', 'DELETE'];
+
+var server = http.createServer(function (req, res) {
+  var code = codes.shift();
+  assert.equal('number', typeof code);
+  assert.ok(code > 0);
+  console.error('writing %d response', code);
+  res.writeHead(code, {});
+  res.end();
+});
+
+var client = http.createClient(common.PORT);
+
+function nextRequest() {
+  var method = methods.shift();
+  console.error("writing request: %s", method);
+
+  var request = client.request(method, '/');
+  request.on('response', function (response) {
+    response.on('end', function() {
+      if (methods.length == 0) {
+        console.error("close server");
+        server.close();
+      } else {
+        // throws error:
+        nextRequest();
+        // works just fine:
+        //process.nextTick(nextRequest);
+      }
+    });
+  });
+  request.end();
+}
+
+server.listen(common.PORT, nextRequest);