Cleanup for hapijs#2429

Eran Hammer · Eran Hammer · commit 3aca2843734f · 2015-03-06T12:41:30.000-08:00
diff --git a/lib/request.js b/lib/request.js
@@ -113,6 +113,7 @@ exports = module.exports = internals.Request = function (connection, req, res, o
 
     this._states = {};
     this._logger = [];
+    this._isPayloadPending = true;      // false when incoming payload fully processed
     this._isBailed = false;             // true when lifecycle should end
     this._isReplied = false;            // true when response processing started
     this._isFinalized = false;          // true when request completed (may be waiting on tails to complete)
@@ -121,11 +122,19 @@ exports = module.exports = internals.Request = function (connection, req, res, o
     this._protect = new Protect(this);
     this.domain = this._protect.domain;
 
-    // Listen to request errors
+    // Listen to request state
+
+    this._onEnd = function () {
+
+        self._isPayloadPending = false;
+    };
+
+    this.raw.req.once('end', this._onEnd);
 
     this._onClose = function () {
 
         self._log(['request', 'closed', 'error']);
+        self._isPayloadPending = false;
         self._isBailed = true;
     };
 
@@ -134,6 +143,7 @@ exports = module.exports = internals.Request = function (connection, req, res, o
     this._onError = function (err) {
 
         self._log(['request', 'error'], err);
+        self._isPayloadPending = false;
     };
 
     this.raw.req.once('error', this._onError);
@@ -387,6 +397,7 @@ internals.Request.prototype._finalize = function () {
 
     // Cleanup
 
+    this.raw.req.removeListener('end', this._onEnd);
     this.raw.req.removeListener('close', this._onClose);
     this.raw.req.removeListener('error', this._onError);
 
diff --git a/lib/route.js b/lib/route.js
@@ -363,7 +363,7 @@ internals.payload = function (request, next) {
         return next();
     }
 
-    Subtext.parse(request.raw.req, request._tap(), request.route.settings.payload, function (err, parsed) {
+    var onParsed = function (err, parsed) {
 
         request.mime = parsed.mime;
         request.payload = parsed.payload || null;
@@ -382,6 +382,23 @@ internals.payload = function (request, next) {
         }
 
         return next();
+    };
+
+    Subtext.parse(request.raw.req, request._tap(), request.route.settings.payload, function (err, parsed) {
+
+        if (!request._isPayloadPending) {
+            return onParsed(err, parsed);
+        }
+
+        // Flush out any pending request payload not consumed due to errors or other restrictions
+
+        request.raw.req.once('end', function () {
+
+            request._isPayloadPending = false;
+            return onParsed(err, parsed);
+        });
+
+        request.raw.req.resume();
     });
 };
 
diff --git a/lib/transmit.js b/lib/transmit.js
@@ -312,8 +312,8 @@ internals.transmit = function (response, callback) {
     var preview = (tap ? source.pipe(tap) : source);
     var compressed = (compressor ? preview.pipe(compressor) : preview);
     var ranged = (ranger ? compressed.pipe(ranger) : compressed);
-    ranged.pipe(request.raw.res, { end: false });
-
+    ranged.pipe(request.raw.res);//, { end: false });
+    /*
     ranged.once('end', function () {
 
         if (request.raw.req.complete === false) {
@@ -322,7 +322,7 @@ internals.transmit = function (response, callback) {
 
         end();
     });
-
+    */
     // Injection
 
     if (Shot.isInjection(request.raw.req)) {
diff --git a/test/payload.js b/test/payload.js
@@ -154,36 +154,25 @@ describe('payload', function () {
         });
     });
 
-    it('returns 400 when payload is not consumed', function (done) {
+    it('returns 400 with response when payload is not consumed', function (done) {
 
         var payload = new Buffer(10 * 1024 * 1024).toString();
 
-        var handler = function (request, reply) {
-
-            expect(request.payload.toString()).to.equal(payload);
-            return reply(request.payload);
-        };
-
         var server = new Hapi.Server();
         server.connection();
-        server.route({ method: 'POST', path: '/', config: { handler: handler, payload: { maxBytes: 10 } } });
+        server.route({ method: 'POST', path: '/', config: { handler: function (request, reply) { return reply(); }, payload: { maxBytes: 1024 * 1024 } } });
 
         server.start(function () {
 
             var uri = 'http://localhost:' + server.info.port;
 
-            Wreck.post(uri, { payload: payload, agent: false }, function (err, res, body) {
+            Wreck.post(uri, { payload: payload }, function (err, res, body) {
 
                 expect(err).to.not.exist();
                 expect(res.statusCode).to.equal(400);
-                expect(body).to.exist();
-                Wreck.post(uri, { payload: payload, agent: false }, function (err, res, body) {
-
-                    expect(err).to.not.exist();
-                    expect(res.statusCode).to.equal(400);
-                    expect(body).to.exist();
-                    done();
-                });
+                expect(body).to.equal('{"statusCode":400,"error":"Bad Request","message":"Payload content length greater than maximum allowed: 1048576"}');
+
+                done();
             });
         });
     });
