🔒️ improve disconnect handling

Author: ctcpip

lib/make-middleware.js

@@ -28,24 +28,8 @@ function makeMiddleware (setup) {
 
     req.body = Object.create(null)
 
-    req.on('error', function (err) {
-      abortWithError(err)
-    })
-
     var busboy
-
-    try {
-      busboy = Busboy({
-        headers: req.headers,
-        limits: limits,
-        preservePath: preservePath,
-        defParamCharset: defParamCharset
-      })
-    } catch (err) {
-      return next(err)
-    }
-
-    var appender = new FileAppender(fileStrategy, req)
+    var appender = null
     var isDone = false
     var readFinished = false
     var errorOccured = false
@@ -55,12 +39,14 @@ function makeMiddleware (setup) {
     function done (err) {
       if (isDone) return
       isDone = true
-      req.unpipe(busboy)
+      if (busboy) {
+        req.unpipe(busboy)
+        setImmediate(() => {
+          busboy.removeAllListeners()
+        })
+      }
       drainStream(req)
       req.resume()
-      setImmediate(() => {
-        busboy.removeAllListeners()
-      })
       next(err)
     }
 
@@ -90,6 +76,38 @@ function makeMiddleware (setup) {
       abortWithError(new MulterError(code, optionalField))
     }
 
+    function handleRequestFailure (err) {
+      if (isDone) return
+      if (busboy) busboy.destroy(err)
+      abortWithError(err)
+    }
+
+    req.on('error', function (err) {
+      handleRequestFailure(err || new Error('Request error'))
+    })
+
+    req.on('aborted', function () {
+      handleRequestFailure(new Error('Request aborted'))
+    })
+
+    req.on('close', function () {
+      if (req.readableEnded) return
+      handleRequestFailure(new Error('Request closed'))
+    })
+
+    try {
+      busboy = Busboy({
+        headers: req.headers,
+        limits: limits,
+        preservePath: preservePath,
+        defParamCharset: defParamCharset
+      })
+    } catch (err) {
+      return next(err)
+    }
+
+    appender = new FileAppender(fileStrategy, req)
+
     // handle text field data
     busboy.on('field', function (fieldname, value, { nameTruncated, valueTruncated }) {
       if (fieldname == null) return abortWithCode('MISSING_FIELD_NAME')

test/error-handling.js

@@ -7,6 +7,8 @@ var util = require('./_util')
 var multer = require('../')
 var stream = require('stream')
 var FormData = require('form-data')
+var http = require('http')
+var net = require('net')
 
 function withLimits (limits, fields) {
   var storage = multer.memoryStorage()
@@ -277,4 +279,52 @@ describe('Error Handling', function () {
       done()
     })
   })
+
+  it('should not hang when client aborts multipart upload', function (done) {
+    this.timeout(5000)
+
+    var upload = multer({ storage: multer.memoryStorage() }).any()
+
+    var server = http.createServer(function (req, res) {
+      var hung = false
+
+      var timer = setTimeout(function () {
+        hung = true
+        server.close()
+        done(new Error('Middleware hung when client aborted request'))
+      }, 1000)
+
+      upload(req, res, function (/* err */) {
+        if (hung) return
+        clearTimeout(timer)
+        server.close()
+        done()
+      })
+    })
+
+    server.listen(0, function () {
+      var port = server.address().port
+      var boundary = 'PoC' + Date.now()
+      var sock = new net.Socket()
+
+      sock.connect(port, '127.0.0.1', function () {
+        sock.write(
+          'POST / HTTP/1.1\r\n' +
+          'Host: localhost\r\n' +
+          'Content-Type: multipart/form-data; boundary=' + boundary + '\r\n' +
+          'Content-Length: 999999\r\n\r\n' +
+          '--' + boundary + '\r\n' +
+          'Content-Disposition: form-data; name="file"; filename="test.bin"\r\n' +
+          'Content-Type: application/octet-stream\r\n\r\n' +
+          'AAAAAAAAAAAAAAAA'
+        )
+
+        setTimeout(function () {
+          sock.destroy()
+        }, 50)
+      })
+
+      sock.on('error', function () {})
+    })
+  })
 })