Upgrade content-disposition

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)

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;
 };

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",

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)
     })
   })

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();
 

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, '<p>{{user.name}}</p>', 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)
       })
     })