From 7f795e4c8b0cbfa84a3cb0126420a50ee34cf905 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 16 May 2023 13:19:10 +0100 Subject: [PATCH] Use content-disposition for setting the Content-Disposition header (#13072) GitOrigin-RevId: f84100beb5e00485be4732c954b5ee553fe18558 --- package-lock.json | 2 +- .../web/app/src/Features/Compile/CompileController.js | 2 +- services/web/app/src/infrastructure/ExpressLocals.js | 11 +++++------ services/web/package.json | 2 +- .../test/unit/src/Compile/CompileControllerTests.js | 6 +++--- .../unit/src/FileStore/FileStoreControllerTests.js | 6 +++--- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 067fefa9db..55d7e6f326 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35192,6 +35192,7 @@ "classnames": "^2.2.6", "codemirror": "~5.33.0", "connect-redis": "^6.1.3", + "content-disposition": "^0.5.0", "contentful": "^6.1.1", "cookie": "^0.2.3", "cookie-parser": "1.3.5", @@ -35334,7 +35335,6 @@ "chai-as-promised": "^7.1.1", "chai-exclude": "^2.0.3", "cheerio": "^1.0.0-rc.3", - "content-disposition": "^0.5.0", "copy-webpack-plugin": "^10.2.4", "css-loader": "^6.7.1", "css-minimizer-webpack-plugin": "^3.4.1", diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index aebd51d03e..0885e91df4 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -339,7 +339,7 @@ module.exports = CompileController = { if (req.query.popupDownload) { res.setContentDisposition('attachment', { filename }) } else { - res.setContentDisposition('', { filename }) + res.setContentDisposition('inline', { filename }) } rateLimit(function (err, canContinue) { diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js index 1c4588d1e7..d369f950f9 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.js +++ b/services/web/app/src/infrastructure/ExpressLocals.js @@ -7,6 +7,7 @@ const { URL } = require('url') const Path = require('path') const moment = require('moment') const request = require('request') +const contentDisposition = require('content-disposition') const Features = require('./Features') const SessionManager = require('../Features/Authentication/SessionManager') const SplitTestMiddleware = require('../Features/SplitTests/SplitTestMiddleware') @@ -104,13 +105,11 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) { }) function addSetContentDisposition(req, res, next) { - res.setContentDisposition = function (type, opts) { - const directives = _.map( - opts, - (v, k) => `${k}="${encodeURIComponent(v)}"` + res.setContentDisposition = function (type, { filename }) { + res.setHeader( + 'Content-Disposition', + contentDisposition(filename, { type }) ) - const contentDispositionValue = `${type}; ${directives.join('; ')}` - res.setHeader('Content-Disposition', contentDispositionValue) } next() } diff --git a/services/web/package.json b/services/web/package.json index 8c75201031..bd47ff43bd 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -147,6 +147,7 @@ "classnames": "^2.2.6", "codemirror": "~5.33.0", "connect-redis": "^6.1.3", + "content-disposition": "^0.5.0", "contentful": "^6.1.1", "cookie": "^0.2.3", "cookie-parser": "1.3.5", @@ -289,7 +290,6 @@ "chai-as-promised": "^7.1.1", "chai-exclude": "^2.0.3", "cheerio": "^1.0.0-rc.3", - "content-disposition": "^0.5.0", "copy-webpack-plugin": "^10.2.4", "css-loader": "^6.7.1", "css-minimizer-webpack-plugin": "^3.4.1", diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 87166a236a..22d9c032b2 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/services/web/test/unit/src/Compile/CompileControllerTests.js @@ -379,9 +379,9 @@ describe('CompileController', function () { }) it('should set the content-disposition header with a safe version of the project name', function () { - this.res.setContentDisposition - .calledWith('', { filename: 'test_nam_.pdf' }) - .should.equal(true) + this.res.setContentDisposition.should.be.calledWith('inline', { + filename: 'test_nam_.pdf', + }) }) it('should increment the pdf-downloads metric', function () { diff --git a/services/web/test/unit/src/FileStore/FileStoreControllerTests.js b/services/web/test/unit/src/FileStore/FileStoreControllerTests.js index 259a583f27..29e22d2e89 100644 --- a/services/web/test/unit/src/FileStore/FileStoreControllerTests.js +++ b/services/web/test/unit/src/FileStore/FileStoreControllerTests.js @@ -81,9 +81,9 @@ describe('FileStoreController', function () { it('should set the Content-Disposition header', function (done) { this.stream.pipe = des => { - this.res.setContentDisposition - .calledWith('attachment', { filename: this.file.name }) - .should.equal(true) + this.res.setContentDisposition.should.be.calledWith('attachment', { + filename: this.file.name, + }) done() } this.controller.getFile(this.req, this.res)