diff --git a/History.md b/History.md index c4d20a1e807..61a79bccd71 100644 --- a/History.md +++ b/History.md @@ -19,6 +19,14 @@ * The default error handler now logs the full error object instead of only its stack trace, so nested details such as `Error.cause` and library-specific properties (e.g. Sequelize's `parent`/`original`) are no longer swallowed - by [@Nitin-Mohapatra](https://github.com/Nitin-Mohapatra) in [#6464](https://github.com/expressjs/express/pull/6464) +* Upgrade `content-disposition` to `^2.0.0`, which changes the `Content-Disposition` header emitted by `res.download()`, `res.attachment()`, and `res.sendFile()`: file names that are valid HTTP tokens are no longer wrapped in quotes. This is equivalent per RFC 6266, but applications asserting on the exact header bytes should update their expectations - by [@blakeembrey](https://github.com/blakeembrey) in [#7233](https://github.com/expressjs/express/pull/7233) + + ```js + res.attachment('user.html'); + // before -> Content-Disposition: attachment; filename="user.html" + // after -> Content-Disposition: attachment; filename=user.html + ``` + ## ⚡ Performance * Avoid duplicate Content-Type header processing in `res.send()` when sending string responses without an explicit Content-Type header - by [@bjohansebas](https://github.com/bjohansebas) in [#6991](https://github.com/expressjs/express/pull/6991) diff --git a/lib/response.js b/lib/response.js index f965e539dd2..ebcf5f0d954 100644 --- a/lib/response.js +++ b/lib/response.js @@ -31,6 +31,7 @@ var cookie = require('cookie'); var send = require('send'); var extname = path.extname; var resolve = path.resolve; +var basename = path.basename; var vary = require('vary'); const { Buffer } = require('node:buffer'); @@ -454,7 +455,7 @@ res.download = function download (path, filename, options, callback) { // set Content-Disposition when file is sent var headers = { - 'Content-Disposition': contentDisposition(name || path) + 'Content-Disposition': contentDisposition.create(basename(name || path)) }; // merge user-provided headers @@ -602,11 +603,12 @@ res.format = function(obj){ */ res.attachment = function attachment(filename) { - if (filename) { - this.type(extname(filename)); + const name = filename !== undefined ? basename(filename) : undefined; + if (name) { + this.type(extname(name)); } - this.set('Content-Disposition', contentDisposition(filename)); + this.set('Content-Disposition', contentDisposition.create(name)); return this; }; diff --git a/package.json b/package.json index 3c166a36cba..f1ab51d0265 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", - "content-disposition": "^1.0.0", + "content-disposition": "^2.0.1", "content-type": "^2.0.0", "cookie": "^0.7.1", "cookie-signature": "^1.2.1", diff --git a/test/acceptance/downloads.js b/test/acceptance/downloads.js index 6db43b351e4..3deed3e0cc7 100644 --- a/test/acceptance/downloads.js +++ b/test/acceptance/downloads.js @@ -15,7 +15,7 @@ describe('downloads', function(){ it('should have a download header', function (done) { request(app) .get('/files/notes/groceries.txt') - .expect('Content-Disposition', 'attachment; filename="groceries.txt"') + .expect('Content-Disposition', 'attachment; filename=groceries.txt') .expect(200, done) }) }) @@ -24,7 +24,7 @@ describe('downloads', function(){ it('should have a download header', function(done){ request(app) .get('/files/amazing.txt') - .expect('Content-Disposition', 'attachment; filename="amazing.txt"') + .expect('Content-Disposition', 'attachment; filename=amazing.txt') .expect(200, done) }) }) diff --git a/test/res.attachment.js b/test/res.attachment.js index 8644bab5b2d..84581bd6c96 100644 --- a/test/res.attachment.js +++ b/test/res.attachment.js @@ -31,7 +31,33 @@ describe('res', function(){ request(app) .get('/') - .expect('Content-Disposition', 'attachment; filename="image.png"', done); + .expect('Content-Disposition', 'attachment; filename=image.png', done); + }) + + it('should quote file names that are not a valid token', function(done){ + var app = express(); + + app.use(function(req, res){ + res.attachment('/path/to/my report.png'); + res.send('foo'); + }); + + request(app) + .get('/') + .expect('Content-Disposition', 'attachment; filename="my report.png"', done); + }) + + it('should handle an empty file name', function(done){ + var app = express(); + + app.use(function(req, res){ + res.attachment(''); + res.send('foo'); + }); + + request(app) + .get('/') + .expect('Content-Disposition', 'attachment; filename=""', done); }) it('should set the Content-Type', function(done){ @@ -63,6 +89,20 @@ describe('res', function(){ .expect(200, done); }) + it('should encode latin1 file names with an ASCII fallback and filename* param', function(done){ + var app = express(); + + app.use(function(req, res){ + res.attachment('/locales/café.txt'); + res.send('coffee'); + }); + + request(app) + .get('/') + .expect('Content-Disposition', 'attachment; filename="caf?.txt"; filename*=UTF-8\'\'caf%C3%A9.txt') + .expect(200, done); + }) + it('should set the Content-Type', function(done){ var app = express(); diff --git a/test/res.download.js b/test/res.download.js index e9966007eba..f059ad13cbf 100644 --- a/test/res.download.js +++ b/test/res.download.js @@ -24,7 +24,7 @@ describe('res', function(){ request(app) .get('/') .expect('Content-Type', 'text/html; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="user.html"') + .expect('Content-Disposition', 'attachment; filename=user.html') .expect(200, '

{{user.name}}

', done) }) @@ -67,7 +67,7 @@ describe('res', function(){ request(app) .get('/') .expect('Content-Type', 'text/html; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .expect(200, done) }) }) @@ -84,7 +84,7 @@ describe('res', function(){ request(app) .get('/') .expect('Content-Type', 'text/html; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="user.html"') + .expect('Content-Disposition', 'attachment; filename=user.html') .expect(200, cb); }) @@ -113,7 +113,7 @@ describe('res', function(){ request(app) .get('/') .expect('Content-Type', 'text/plain; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="name.txt"') + .expect('Content-Disposition', 'attachment; filename=name.txt') .expect(200, 'tobi', cb) }) @@ -162,7 +162,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename=".name"') + .expect('Content-Disposition', 'attachment; filename=.name') .expect('Cache-Control', 'public, max-age=14400') .expect(utils.shouldHaveBody(Buffer.from('tobi'))) .end(done) @@ -259,7 +259,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename="user.html"') + .expect('Content-Disposition', 'attachment; filename=user.html') .end(done) }) @@ -277,7 +277,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename="user.html"') + .expect('Content-Disposition', 'attachment; filename=user.html') .end(done) }) }) @@ -296,7 +296,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename="name.txt"') + .expect('Content-Disposition', 'attachment; filename=name.txt') .expect(utils.shouldHaveBody(Buffer.from('tobi'))) .end(done) }) @@ -313,7 +313,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename="name.txt"') + .expect('Content-Disposition', 'attachment; filename=name.txt') .expect(utils.shouldHaveBody(Buffer.from('tobi'))) .end(done) }) @@ -367,7 +367,7 @@ describe('res', function(){ request(app) .get('/') .expect('Content-Type', 'text/html; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .expect(200, cb); }) }) @@ -386,7 +386,7 @@ describe('res', function(){ .get('/') .expect(200) .expect('Content-Type', 'text/html; charset=utf-8') - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .end(cb) }) @@ -403,7 +403,7 @@ describe('res', function(){ request(app) .get('/') .expect(200) - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .expect('Cache-Control', 'public, max-age=14400') .expect(utils.shouldHaveBody(Buffer.from('tobi'))) .end(done) @@ -426,7 +426,7 @@ describe('res', function(){ .get('/') .expect(200) .expect('Content-Type', 'text/x-custom') - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .end(done) }) @@ -446,7 +446,7 @@ describe('res', function(){ .get('/') .expect(200) .expect('Content-Type', 'text/x-custom') - .expect('Content-Disposition', 'attachment; filename="document"') + .expect('Content-Disposition', 'attachment; filename=document') .end(done) }) })