fix(bodyReader): Fix memory leak (#1566)

Without handling the req 'close' event, we will leak ClientRequest objects when the client suddently stops sending the request body.

lib/plugins/bodyReader.js

@@ -6,6 +6,7 @@ var crypto = require('crypto');
 var zlib = require('zlib');
 
 var assert = require('assert-plus');
+var once = require('once');
 var errors = require('restify-errors');
 
 ///--- Globals
@@ -67,7 +68,9 @@ function bodyReader(options) {
 
     var maxBodySize = opts.maxBodySize || 0;
 
-    function readBody(req, res, next) {
+    function readBody(req, res, originalNext) {
+        var next = once(originalNext);
+
         // #100 don't read the body again if we've read it once
         if (req._readBody) {
             next();
@@ -172,6 +175,11 @@ function bodyReader(options) {
         });
 
         req.once('error', next);
+        // add 'close and 'aborted' event handlers so that requests (and their
+        // corresponding memory) don't leak if client stops sending data half
+        // way through a POST request
+        req.once('close', next);
+        req.once('aborted', next);
         req.resume();
     }
 

package.json

@@ -73,7 +73,7 @@
     "express",
     "DTrace"
   ],
-  "version": "6.3.2",
+  "version": "6.3.3",
   "repository": {
     "type": "git",
     "url": "git://github.com/restify/node-restify.git"

test/plugins/bodyReader.js → test/plugins/bodyReader.test.js

@@ -2,6 +2,7 @@
 /* eslint-disable func-names */
 
 // core requires
+var http = require('http');
 
 // external requires
 var assert = require('chai').assert;
@@ -121,5 +122,64 @@ describe('body reader', function() {
                 }
             );
         });
+
+        it('should handle client timeout', function(done) {
+            SERVER.use(restify.plugins.bodyParser());
+
+            SERVER.post('/compressed', function(req, res, next) {
+                res.send(200, { inflightRequests: SERVER.inflightRequests() });
+                next();
+            });
+
+            // set timeout to 100ms so test runs faster, when client stops
+            // sending POST data
+            SERVER.on('connection', function(socket) {
+                socket.setTimeout(100);
+            });
+
+            var postData = 'hello world';
+
+            var options = {
+                hostname: '127.0.0.1',
+                port: PORT,
+                path: '/compressed',
+                method: 'POST',
+                headers: {
+                    'Content-Type': 'application/json',
+                    // report postData + 1 so that request isn't sent
+                    'Content-Length': Buffer.byteLength(postData) + 1
+                }
+            };
+
+            var req = http.request(options, function(res) {
+                // should never receive a response
+                assert.isNotOk(res);
+            });
+
+            // will get a req error after 100ms timeout
+            req.on('error', function(e) {
+                // make another request to verify in flight request is only 1
+                CLIENT = restifyClients.createJsonClient({
+                    url: 'http://127.0.0.1:' + PORT,
+                    retry: false
+                });
+
+                CLIENT.post(
+                    '/compressed',
+                    {
+                        apple: 'red'
+                    },
+                    function(err, _, res, obj) {
+                        assert.ifError(err);
+                        assert.equal(res.statusCode, 200);
+                        assert.equal(obj.inflightRequests, 1);
+                        done();
+                    }
+                );
+            });
+
+            // write data to request body, but don't req.send()
+            req.write(postData);
+        });
     });
 });