From a9b8b864df70c5497a928b2d54937f0c3f0bbfcb Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 12 Apr 2017 16:00:02 +0100 Subject: [PATCH] Move content-disposition setting into a method on `res` --- .../Features/Compile/CompileController.coffee | 6 ++---- .../Downloads/ProjectDownloadsController.coffee | 12 ++++++------ .../Features/FileStore/FileStoreController.coffee | 2 +- .../app/coffee/infrastructure/ExpressLocals.coffee | 13 +++++++++++++ .../coffee/Compile/CompileControllerTests.coffee | 4 ++-- .../ProjectDownloadsControllerTests.coffee | 12 ++++++------ .../FileStore/FileStoreControllerTests.coffee | 5 +++-- .../UnitTests/coffee/helpers/MockResponse.coffee | 2 ++ 8 files changed, 35 insertions(+), 21 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 5eb67540e4..6d893f0dbb 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -85,12 +85,10 @@ module.exports = CompileController = res.contentType("application/pdf") if !!req.query.popupDownload logger.log project_id: project_id, "download pdf as popup download" - res.header( - 'Content-Disposition', "attachment; filename=#{encodeURIComponent(project.getSafeProjectName())}.pdf" - ) + res.setContentDisposition('attachment', {filename: "#{project.getSafeProjectName()}.pdf"}) else logger.log project_id: project_id, "download pdf to embed in browser" - res.header('Content-Disposition', "filename=#{project.getSafeProjectName()}.pdf") + res.setContentDisposition('', {filename: "#{project.getSafeProjectName()}.pdf"}) rateLimit (err, canContinue)-> if err? diff --git a/services/web/app/coffee/Features/Downloads/ProjectDownloadsController.coffee b/services/web/app/coffee/Features/Downloads/ProjectDownloadsController.coffee index b488472149..e36c35d639 100644 --- a/services/web/app/coffee/Features/Downloads/ProjectDownloadsController.coffee +++ b/services/web/app/coffee/Features/Downloads/ProjectDownloadsController.coffee @@ -15,9 +15,9 @@ module.exports = ProjectDownloadsController = return next(error) if error? ProjectZipStreamManager.createZipStreamForProject project_id, (error, stream) -> return next(error) if error? - res.header( - "Content-Disposition", - "attachment; filename=#{encodeURIComponent(project.name)}.zip" + res.setContentDisposition( + 'attachment', + {filename: "#{project.name}.zip"} ) res.contentType('application/zip') stream.pipe(res) @@ -30,9 +30,9 @@ module.exports = ProjectDownloadsController = return next(error) if error? ProjectZipStreamManager.createZipStreamForMultipleProjects project_ids, (error, stream) -> return next(error) if error? - res.header( - "Content-Disposition", - "attachment; filename=ShareLaTeX Projects (#{project_ids.length} items).zip" + res.setContentDisposition( + 'attachment', + {filename: "ShareLaTeX Projects (#{project_ids.length} items).zip"} ) res.contentType('application/zip') stream.pipe(res) diff --git a/services/web/app/coffee/Features/FileStore/FileStoreController.coffee b/services/web/app/coffee/Features/FileStore/FileStoreController.coffee index 9fe80b9108..eceb5aee4f 100644 --- a/services/web/app/coffee/Features/FileStore/FileStoreController.coffee +++ b/services/web/app/coffee/Features/FileStore/FileStoreController.coffee @@ -35,5 +35,5 @@ module.exports = if (is_mobile_safari(user_agent) and is_html(file)) logger.log filename: file.name, user_agent: user_agent, "sending html file to mobile-safari as plain text" res.setHeader('Content-Type', 'text/plain') - res.setHeader("Content-Disposition", "attachment; filename=#{encodeURIComponent(file.name)}") + res.setContentDisposition('attachment', {filename: file.name}) stream.pipe res diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 8124d13d93..ba753cc86e 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -71,6 +71,19 @@ module.exports = (app, webRouter, apiRouter)-> res.locals.session = req.session next() + addSetContentDisposition = (req, res, next) -> + res.setContentDisposition = (type, opts) -> + directives = for k, v of opts + "#{k}=\"#{encodeURIComponent(v)}\"" + contentDispositionValue = "#{type}; #{directives.join('; ')}" + res.setHeader( + 'Content-Disposition', + contentDispositionValue + ) + next() + webRouter.use addSetContentDisposition + apiRouter.use addSetContentDisposition + webRouter.use (req, res, next)-> cdnBlocked = req.query.nocdn == 'true' or req.session.cdnBlocked diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 33270ec1c7..905cf5ce74 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -136,8 +136,8 @@ describe "CompileController", -> .should.equal true it "should set the content-disposition header with the project name", -> - @res.header - .calledWith("Content-Disposition", "filename=#{encodeURIComponent(@safe_name)}.pdf") + @res.setContentDisposition + .calledWith('', {filename: "#{@safe_name}.pdf"}) .should.equal true it "should increment the pdf-downloads metric", -> diff --git a/services/web/test/UnitTests/coffee/Downloads/ProjectDownloadsControllerTests.coffee b/services/web/test/UnitTests/coffee/Downloads/ProjectDownloadsControllerTests.coffee index 7fd0b81ec8..44cc3916cd 100644 --- a/services/web/test/UnitTests/coffee/Downloads/ProjectDownloadsControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Downloads/ProjectDownloadsControllerTests.coffee @@ -59,10 +59,10 @@ describe "ProjectDownloadsController", -> @Project.findById.calledWith(@project_id, "name").should.equal(true) it "should name the downloaded file after the project", -> - @res.header + @res.setContentDisposition .calledWith( - "Content-Disposition", - "attachment; filename=#{encodeURIComponent(@project_name)}.zip") + 'attachment', + {filename: "#{@project_name}.zip"}) .should.equal true it "should record the action via Metrics", -> @@ -107,10 +107,10 @@ describe "ProjectDownloadsController", -> .should.equal true it "should name the downloaded file after the project", -> - @res.header + @res.setContentDisposition .calledWith( - "Content-Disposition", - "attachment; filename=ShareLaTeX Projects (2 items).zip") + 'attachment', + {filename: "ShareLaTeX Projects (2 items).zip"}) .should.equal true it "should record the action via Metrics", -> diff --git a/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee b/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee index 0bf16b491b..3b3f546e9f 100644 --- a/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee @@ -29,6 +29,7 @@ describe "FileStoreController", -> get: (key) -> undefined @res = setHeader: sinon.stub() + setContentDisposition: sinon.stub() @file = name: "myfile.png" @@ -62,8 +63,8 @@ describe "FileStoreController", -> it "should set the Content-Disposition header", (done)-> @stream.pipe = (des)=> - @res.setHeader.calledWith( - "Content-Disposition", "attachment; filename=#{encodeURIComponent(@file.name)}" + @res.setContentDisposition.calledWith( + "attachment", {filename: @file.name} ).should.equal true done() @controller.getFile @req, @res diff --git a/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee b/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee index 5ed7c58522..d7fbb76a69 100644 --- a/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee +++ b/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee @@ -70,6 +70,8 @@ class MockResponse setHeader: (header, value) -> @headers[header] = value + setContentDisposition: sinon.stub() + setTimeout: (@timout)-> header: sinon.stub()