diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index c7e2c00949..246a9e646e 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -239,6 +239,7 @@ module.exports = ProjectController = hasSubscription: results.hasSubscription isShowingV1Projects: results.v1Projects? warnings: warnings + zipFileSizeLimit: Settings.maxUploadSize } if Settings?.algolia?.app_id? and Settings?.algolia?.read_only_api_key? diff --git a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee index b923431f41..fe41454206 100644 --- a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee @@ -5,6 +5,7 @@ Path = require "path" fse = require "fs-extra" yauzl = require "yauzl" Settings = require "settings-sharelatex" +Errors = require "../Errors/Errors" _ = require("underscore") ONE_MEG = 1024 * 1024 @@ -16,7 +17,7 @@ module.exports = ArchiveManager = totalSizeInBytes = null yauzl.open source, {lazyEntries: true}, (err, zipfile) -> - return callback(err) if err? + return callback(new Errors.InvalidError("invalid_zip_file")) if err? if Settings.maxEntitiesPerProject? and zipfile.entryCount > Settings.maxEntitiesPerProject return callback(null, true) # too many files in zip file @@ -113,7 +114,7 @@ module.exports = ArchiveManager = return callback(err) if isTooLarge - return callback(new Error("zip_too_large")) + return callback(new Errors.InvalidError("zip_contents_too_large")) timer = new metrics.Timer("unzipDirectory") logger.log source: source, destination: destination, "unzipping file" diff --git a/services/web/app/coffee/Features/Uploads/ProjectUploadController.coffee b/services/web/app/coffee/Features/Uploads/ProjectUploadController.coffee index 3cd7019ca0..86cd8985f6 100644 --- a/services/web/app/coffee/Features/Uploads/ProjectUploadController.coffee +++ b/services/web/app/coffee/Features/Uploads/ProjectUploadController.coffee @@ -5,6 +5,22 @@ Path = require "path" FileSystemImportManager = require "./FileSystemImportManager" ProjectUploadManager = require "./ProjectUploadManager" AuthenticationController = require('../Authentication/AuthenticationController') +Settings = require "settings-sharelatex" +Errors = require "../Errors/Errors" +multer = require('multer') + +upload = null + +try + upload = multer( + dest: Settings.path.uploadFolder + limits: fileSize: Settings.maxUploadSize + ) +catch err + if err.message == "EEXIST" + logger.log uploadFolder:Settings.path.uploadFolder, "dir already exists, continuing" + else + logger.err err:err, "caught error from multer in uploads router" module.exports = ProjectUploadController = uploadProject: (req, res, next) -> @@ -19,7 +35,10 @@ module.exports = ProjectUploadController = logger.error err: error, file_path: path, file_name: name, "error uploading project" - res.send success: false + if error.name? && error.name == 'InvalidError' + res.status(422).json { success: false, error: req.i18n.translate(error.message) } + else + res.status(500).json { success: false, error: req.i18n.translate("upload_failed") } else logger.log project: project._id, file_path: path, file_name: name, @@ -52,3 +71,11 @@ module.exports = ProjectUploadController = project_id: project_id, file_path: path, file_name: name, folder_id: folder_id "uploaded file" res.send success: true, entity_id: entity?._id, entity_type: entity?.type + + multerMiddleware: (req, res, next) -> + return res.status(500).json {success: false, error: req.i18n.translate("upload_failed")} unless upload? + upload.single('qqfile') req, res, (err) -> + if err instanceof multer.MulterError && err.code == 'LIMIT_FILE_SIZE' + return res.status(422).json {success: false, error: req.i18n.translate("file_too_large")} + + next(err) diff --git a/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee b/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee index c47917774a..6ffaa4212e 100644 --- a/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee +++ b/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee @@ -3,19 +3,6 @@ AuthenticationController = require('../Authentication/AuthenticationController') ProjectUploadController = require "./ProjectUploadController" RateLimiterMiddleware = require('../Security/RateLimiterMiddleware') Settings = require('settings-sharelatex') -multer = require('multer') - -try - upload = multer( - dest: Settings.path.uploadFolder - limits: fileSize: Settings.maxUploadSize - ) -catch err - if err.message == "EEXIST" - logger.log uploadFolder:Settings.path.uploadFolder, "dir already exists, continuing" - else - logger.err err:err, "caught error from multer in uploads router" - module.exports = apply: (webRouter, apiRouter) -> @@ -26,7 +13,7 @@ module.exports = maxRequests: 20 timeInterval: 60 }), - upload.single('qqfile'), + ProjectUploadController.multerMiddleware, ProjectUploadController.uploadProject webRouter.post '/Project/:Project_id/upload', @@ -38,5 +25,5 @@ module.exports = }), AuthenticationController.requireLogin(), AuthorizationMiddleware.ensureUserCanWriteProjectContent, - upload.single('qqfile'), + ProjectUploadController.multerMiddleware, ProjectUploadController.uploadFile diff --git a/services/web/app/views/project/list/modals.pug b/services/web/app/views/project/list/modals.pug index d01909c94a..f0dc4bbb78 100644 --- a/services/web/app/views/project/list/modals.pug +++ b/services/web/app/views/project/list/modals.pug @@ -256,6 +256,7 @@ script(type="text/ng-template", id="uploadProjectModalTemplate") endpoint="/project/new/upload" template-id="qq-project-uploader-template" multiple="false" + size-limit=zipFileSizeLimit allowed-extensions="['zip']" on-complete-callback="onComplete" ) diff --git a/services/web/public/src/directives/fineUpload.js b/services/web/public/src/directives/fineUpload.js index 5e402750c1..41b4d7adc4 100644 --- a/services/web/public/src/directives/fineUpload.js +++ b/services/web/public/src/directives/fineUpload.js @@ -15,6 +15,7 @@ define(['base', 'fineuploader'], (App, qq) => multiple: '=', endpoint: '@', templateId: '@', + sizeLimit: '@', allowedExtensions: '=', onCompleteCallback: '=', onUploadCallback: '=', @@ -36,6 +37,9 @@ define(['base', 'fineuploader'], (App, qq) => } else { validation = {} } + if (scope.sizeLimit) { + validation.sizeLimit = scope.sizeLimit + } const maxConnections = scope.maxConnections || 1 const onComplete = scope.onCompleteCallback || function() {} const onUpload = scope.onUploadCallback || function() {} @@ -72,7 +76,11 @@ define(['base', 'fineuploader'], (App, qq) => onSubmit, onCancel }, - template: templateId + template: templateId, + failedUploadTextDisplay: { + mode: 'custom', + responseProperty: 'error' + } }) window.q = q if (scope.control != null) { diff --git a/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee b/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee index 66c2596c9d..4c0208d559 100644 --- a/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee +++ b/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee @@ -3,6 +3,7 @@ expect = require("chai").expect chai = require('chai') should = chai.should() modulePath = "../../../../app/js/Features/Uploads/ArchiveManager.js" +Errors = require("../../../../app/js/Features/Errors/Errors") SandboxedModule = require('sandboxed-module') events = require "events" @@ -50,13 +51,13 @@ describe "ArchiveManager", -> describe "with an error in the zip file header", -> beforeEach (done) -> - @yauzl.open = sinon.stub().callsArgWith(2, new Error("Something went wrong")) + @yauzl.open = sinon.stub().callsArgWith(2, new Errors.InvalidError("invalid_zip_file")) @ArchiveManager.extractZipArchive @source, @destination, (error) => @callback(error) done() it "should return the callback with an error", -> - @callback.calledWithExactly(new Error("Something went wrong")).should.equal true + sinon.assert.calledWithExactly(@callback, new Errors.InvalidError("invalid_zip_file")) it "should log out the error", -> @logger.error.called.should.equal true @@ -69,7 +70,7 @@ describe "ArchiveManager", -> done() it "should return the callback with an error", -> - @callback.calledWithExactly(new Error("zip_too_large")).should.equal true + sinon.assert.calledWithExactly(@callback, new Errors.InvalidError("zip_contents_too_large")) it "should not call yauzl.open", -> @yauzl.open.called.should.equal false diff --git a/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee b/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee index cabc170b20..b4065e5774 100644 --- a/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee +++ b/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee @@ -6,6 +6,7 @@ modulePath = "../../../../app/js/Features/Uploads/ProjectUploadController.js" SandboxedModule = require('sandboxed-module') MockRequest = require "../helpers/MockRequest" MockResponse = require "../helpers/MockResponse" +Errors = require("../../../../app/js/Features/Errors/Errors") describe "ProjectUploadController", -> beforeEach -> @@ -88,8 +89,24 @@ describe "ProjectUploadController", -> @ProjectUploadController.uploadProject @req, @res it "should return a failed response to the FileUploader client", -> - expect(@res.body).to.deep.equal - success: false + expect(@res.body).to.deep.equal JSON.stringify({ success: false, error: "upload_failed" }) + + it "should output an error log line", -> + @logger.error + .calledWith(sinon.match.any, "error uploading project") + .should.equal true + + describe "when ProjectUploadManager.createProjectFromZipArchive reports the file as invalid", -> + beforeEach -> + @ProjectUploadManager.createProjectFromZipArchive = + sinon.stub().callsArgWith(3, new Errors.InvalidError("zip_contents_too_large"), @project) + @ProjectUploadController.uploadProject @req, @res + + it "should return the reported error to the FileUploader client", -> + expect(@res.body).to.deep.equal JSON.stringify({ success: false, error: "zip_contents_too_large" }) + + it "should return an 'unprocessable entity' status code", -> + expect(@res.statusCode).to.equal 422 it "should output an error log line", -> @logger.error diff --git a/services/web/test/unit/coffee/helpers/MockRequest.coffee b/services/web/test/unit/coffee/helpers/MockRequest.coffee index 51208dba11..aea1b8deda 100644 --- a/services/web/test/unit/coffee/helpers/MockRequest.coffee +++ b/services/web/test/unit/coffee/helpers/MockRequest.coffee @@ -8,7 +8,7 @@ class MockRequest body: {} _parsedUrl:{} i18n: - translate:-> + translate: (str)-> str route: path: '' diff --git a/services/web/test/unit/coffee/helpers/MockResponse.coffee b/services/web/test/unit/coffee/helpers/MockResponse.coffee index 7b2c8e88cd..89c0b9f580 100644 --- a/services/web/test/unit/coffee/helpers/MockResponse.coffee +++ b/services/web/test/unit/coffee/helpers/MockResponse.coffee @@ -53,7 +53,7 @@ class MockResponse if arguments.length < 2 if typeof status != "number" body = status - status = 200 + status = @statusCode || 200 @statusCode = status @returned = true @type = 'application/json' @@ -64,7 +64,8 @@ class MockResponse @body = JSON.stringify(body) if body @callback() if @callback? - status: (@statusCode)-> + status: (status)-> + @statusCode = status return @