From 029077fe6e3ffd01665bbe424880f0746c0b1488 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 20 Feb 2014 22:33:12 +0000 Subject: [PATCH] downloading a file now sets the filename in header correctly --- .../FileStore/FileStoreController.coffee | 21 ++++++ .../controllers/ProjectController.coffee | 10 --- services/web/app/coffee/router.coffee | 3 +- .../FileStore/FileStoreControllerTests.coffee | 68 +++++++++++++++++++ 4 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 services/web/app/coffee/Features/FileStore/FileStoreController.coffee create mode 100644 services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee diff --git a/services/web/app/coffee/Features/FileStore/FileStoreController.coffee b/services/web/app/coffee/Features/FileStore/FileStoreController.coffee new file mode 100644 index 0000000000..8182268d8e --- /dev/null +++ b/services/web/app/coffee/Features/FileStore/FileStoreController.coffee @@ -0,0 +1,21 @@ +logger = require('logger-sharelatex') +FileStoreHandler = require("./FileStoreHandler") +ProjectLocator = require("../Project/ProjectLocator") + +module.exports = + + getFile : (req, res)-> + project_id = req.params.Project_id + file_id = req.params.File_id + queryString = req.query + logger.log project_id: project_id, file_id: file_id, queryString:queryString, "file download" + ProjectLocator.findElement {project_id: project_id, element_id: file_id, type: "file"}, (err, file)-> + if err? + logger.err err:err, project_id: project_id, file_id: file_id, queryString:queryString, "error finding element for downloading file" + return res.send 500 + FileStoreHandler.getFileStream project_id, file_id, queryString, (err, stream)-> + if err? + logger.err err:err, project_id: project_id, file_id: file_id, queryString:queryString, "error getting file stream for downloading file" + return res.send 500 + res.setHeader("Content-Disposition", "attachment; filename=#{file.name}") + stream.pipe res \ No newline at end of file diff --git a/services/web/app/coffee/controllers/ProjectController.coffee b/services/web/app/coffee/controllers/ProjectController.coffee index c3f80a2bdd..1aefeabd0a 100755 --- a/services/web/app/coffee/controllers/ProjectController.coffee +++ b/services/web/app/coffee/controllers/ProjectController.coffee @@ -10,7 +10,6 @@ SecurityManager = require '../managers/SecurityManager' GuidManager = require '../managers/GuidManager' Settings = require('settings-sharelatex') projectCreationHandler = require '../Features/Project/ProjectCreationHandler' -projectLocator = require '../Features/Project/ProjectLocator' projectDuplicator = require('../Features/Project/ProjectDuplicator') ProjectZipStreamManager = require '../Features/Downloads/ProjectZipStreamManager' metrics = require('../infrastructure/Metrics') @@ -184,15 +183,6 @@ module.exports = class ProjectController @emit "end" if @endEmitted next() - downloadImageFile : (req, res)-> - project_id = req.params.Project_id - file_id = req.params.File_id - queryString = req.query - logger.log project_id: project_id, file_id: file_id, queryString:queryString, "file download" - res.setHeader("Content-Disposition", "attachment") - FileStoreHandler.getFileStream project_id, file_id, queryString, (err, stream)-> - stream.pipe res - cloneProject: (req, res)-> metrics.inc "cloned-project" project_id = req.params.Project_id diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 100902cdb8..8e9e8d4a3d 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -31,6 +31,7 @@ CompileManager = require("./Features/Compile/CompileManager") CompileController = require("./Features/Compile/CompileController") HealthCheckController = require("./Features/HealthCheck/HealthCheckController") ProjectDownloadsController = require "./Features/Downloads/ProjectDownloadsController" +FileStoreController = require("./Features/FileStore/FileStoreController") logger = require("logger-sharelatex") httpAuth = require('express').basicAuth (user, pass)-> @@ -97,7 +98,7 @@ module.exports = class Router app.get '/project/new/template', TemplatesMiddlewear.saveTemplateDataInSession, AuthenticationController.requireLogin(), TemplatesController.createProjectFromZipTemplate app.get '/Project/:Project_id', SecutiryManager.requestCanAccessProject, Project.loadEditor - app.get '/Project/:Project_id/file/:File_id', SecutiryManager.requestCanAccessProject, Project.downloadImageFile + app.get '/Project/:Project_id/file/:File_id', SecutiryManager.requestCanAccessProject, FileStoreController.getFile # This is left for legacy reasons and can be removed once all editors have had a chance to refresh: app.get '/Project/:Project_id/download/pdf', SecutiryManager.requestCanAccessProject, CompileController.downloadPdf diff --git a/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee b/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee new file mode 100644 index 0000000000..fc665cee7a --- /dev/null +++ b/services/web/test/UnitTests/coffee/FileStore/FileStoreControllerTests.coffee @@ -0,0 +1,68 @@ +assert = require("chai").assert +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/Features/FileStore/FileStoreController.js" +SandboxedModule = require('sandboxed-module') + +describe "FileStoreController", -> + + beforeEach -> + @FileStoreHandler = + getFileStream: sinon.stub() + @ProjectLocator = + findElement: sinon.stub() + @controller = SandboxedModule.require modulePath, requires: + "settings-sharelatex": @settings + "logger-sharelatex" : @logger = {log:sinon.stub(), err:sinon.stub()} + "../Project/ProjectLocator": @ProjectLocator + "./FileStoreHandler": @FileStoreHandler + @stream = {} + @project_id = "2k3j1lk3j21lk3j" + @file_id = "12321kklj1lk3jk12" + @req = + params: + Project_id: @project_id + File_id: @file_id + query: "query string here" + @res = + setHeader: sinon.stub() + @file = + name: "myfile.png" + + describe "getFile", -> + + beforeEach -> + @FileStoreHandler.getFileStream.callsArgWith(3, null, @stream) + @ProjectLocator.findElement.callsArgWith(1, null, @file) + + it "should call the file store handler with the project_id file_id and any query string", (done)-> + @stream.pipe = (des)=> + @FileStoreHandler.getFileStream.calledWith(@req.params.Project_id, @req.params.File_id, @req.query).should.equal true + done() + @controller.getFile @req, @res + + it "should pipe to res", (done)-> + @stream.pipe = (des)=> + des.should.equal @res + done() + @controller.getFile @req, @res + + it "should get the file from the db", (done)-> + @stream.pipe = (des)=> + opts = + project_id: @project_id + element_id: @file_id + type: "file" + @ProjectLocator.findElement.calledWith(opts).should.equal true + done() + @controller.getFile @req, @res + + it "should set the Content-Disposition header", (done)-> + @stream.pipe = (des)=> + @res.setHeader.calledWith("Content-Disposition", "attachment; filename=#{@file.name}").should.equal true + done() + @controller.getFile @req, @res + +