fix: decode WHATWG-escaped characters in file originalname (#836)

lib/make-middleware.js

@@ -13,6 +13,23 @@ function drainStream (stream) {
   })
 }
 
+// The WHATWG HTML spec requires user agents to escape the bytes 0x0A (LF),
+// 0x0D (CR) and 0x22 (") as %0A, %0D and %22 when serialising field names and
+// filenames in a multipart/form-data body. Busboy leaves them escaped, so we
+// reverse exactly those three sequences here to recover the original name.
+// Only these are decoded on purpose: a bare `%` is never escaped by the spec,
+// so a full decodeURIComponent would corrupt legitimate names like `50%.pdf`.
+// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart-form-data
+function decodeFormDataName (str) {
+  return str.replace(/%0A|%0D|%22/gi, function (match) {
+    switch (match.toUpperCase()) {
+      case '%0A': return '\n'
+      case '%0D': return '\r'
+      default: return '"'
+    }
+  })
+}
+
 function makeMiddleware (setup) {
   return function multerMiddleware (req, res, next) {
     if (!is(req, ['multipart'])) return next()
@@ -173,7 +190,7 @@ function makeMiddleware (setup) {
 
       var file = {
         fieldname: fieldname,
-        originalname: filename,
+        originalname: decodeFormDataName(filename),
         encoding: encoding,
         mimetype: mimeType
       }

test/filename-decoding.js

@@ -0,0 +1,67 @@
+/* eslint-env mocha */
+
+var assert = require('assert')
+
+var multer = require('../')
+var stream = require('stream')
+
+function submitFilename (filename, cb) {
+  var req = new stream.PassThrough()
+  var boundary = 'AaB03x'
+  var body = [
+    '--' + boundary,
+    'Content-Disposition: form-data; name="file"; filename="' + filename + '"',
+    'Content-Type: text/plain',
+    '',
+    'test file content',
+    '--' + boundary + '--'
+  ].join('\r\n')
+
+  req.headers = {
+    'content-type': 'multipart/form-data; boundary=' + boundary,
+    'content-length': body.length
+  }
+
+  req.end(body)
+
+  multer().single('file')(req, null, function (err) {
+    if (err) return cb(err)
+    cb(null, req.file)
+  })
+}
+
+describe('Filename decoding', function () {
+  it('should decode an escaped double quote (%22)', function (done) {
+    submitFilename('file%22.ext', function (err, file) {
+      assert.ifError(err)
+      assert.strictEqual(file.originalname, 'file".ext')
+      done()
+    })
+  })
+
+  it('should decode escaped CR and LF (%0D, %0A)', function (done) {
+    submitFilename('a%0D%0Ab.ext', function (err, file) {
+      assert.ifError(err)
+      assert.strictEqual(file.originalname, 'a\r\nb.ext')
+      done()
+    })
+  })
+
+  it('should not alter a filename with no escapes', function (done) {
+    submitFilename('hello world.ext', function (err, file) {
+      assert.ifError(err)
+      assert.strictEqual(file.originalname, 'hello world.ext')
+      done()
+    })
+  })
+
+  it('should preserve a literal percent sign', function (done) {
+    // `%` itself is never escaped by the WHATWG serialiser, so a name like
+    // this must survive untouched -- a full decodeURIComponent would break it.
+    submitFilename('50%off.ext', function (err, file) {
+      assert.ifError(err)
+      assert.strictEqual(file.originalname, '50%off.ext')
+      done()
+    })
+  })
+})