diff --git a/lib/make-middleware.js b/lib/make-middleware.js index ee509886..0f48d1cc 100644 --- a/lib/make-middleware.js +++ b/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 } diff --git a/test/filename-decoding.js b/test/filename-decoding.js new file mode 100644 index 00000000..d4b22482 --- /dev/null +++ b/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() + }) + }) +})