From 8812ff445e385694c10166371988f2e58fa94e76 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Sat, 12 Mar 2016 15:43:16 +0000 Subject: [PATCH] change zip size check to spawn --- .../Features/Uploads/ArchiveManager.coffee | 60 +++++++++++++++---- .../coffee/Uploads/ArchiveManagerTests.coffee | 48 ++++++++++++++- 2 files changed, 95 insertions(+), 13 deletions(-) diff --git a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee index 74f6108ba0..a615810fe6 100644 --- a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee @@ -3,30 +3,68 @@ logger = require "logger-sharelatex" metrics = require "../../infrastructure/Metrics" fs = require "fs" Path = require "path" +_ = require("underscore") ONE_MEG = 1024 * 1024 module.exports = ArchiveManager = + + + _isZipTooLarge: (source, callback = (err, isTooLarge)->)-> + callback = _.once callback + + unzip = child.spawn("unzip", ["-l", source]) + + output = "" + unzip.stdout.on "data", (d)-> + output += d + + error = null + unzip.stderr.on "data", (chunk) -> + error ||= "" + error += chunk + + unzip.on "error", (err) -> + logger.error {err, source, destination}, "unzip failed" + if err.code == "ENOENT" + logger.error "unzip command not found. Please check the unzip command is installed" + callback(err) + + unzip.on "exit", () -> + if error? + error = new Error(error) + logger.error err:error, source: source, destination: destination, "error checking zip size" + + lines = output.split("\n") + lastLine = lines[lines.length - 2]?.trim() + totalSizeInBytes = lastLine?.split(" ")?[0] + + totalSizeInBytes = parseInt(totalSizeInBytes) + + if !totalSizeInBytes? or isNaN(totalSizeInBytes) + logger.err source:source, "error getting bytes of zip" + return callback(new Error("something went wrong")) + + isTooLarge = totalSizeInBytes > (ONE_MEG * 300) + + callback(error, isTooLarge) + + + + + extractZipArchive: (source, destination, _callback = (err) ->) -> callback = (args...) -> _callback(args...) _callback = () -> - child.exec "unzip -l #{source} | tail -n 1", (err, result)-> + ArchiveManager._isZipTooLarge source, (err, isTooLarge)-> if err? logger.err err:err, "error checking size of zip file" return callback(err) - totalSizeInBytes = result.trim()?.split(" ")?[0] - - if !totalSizeInBytes? - logger.err source:source, "error getting bytes of zip" - return callback(new Error("something went wrong")) - - if totalSizeInBytes > ONE_MEG * 300 - logger.log source:source, totalSizeInBytes:totalSizeInBytes, "zip file too large" - return callback(new Error("zip_too_large")) - + if isTooLarge + return callback(new Error("zip_too_large")) timer = new metrics.Timer("unzipDirectory") diff --git a/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee b/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee index f1547d0413..89b14c78c9 100644 --- a/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Uploads/ArchiveManagerTests.coffee @@ -1,4 +1,5 @@ sinon = require('sinon') +expect = require("chai").expect chai = require('chai') should = chai.should() modulePath = "../../../../app/js/Features/Uploads/ArchiveManager.js" @@ -9,13 +10,16 @@ describe "ArchiveManager", -> beforeEach -> @logger = error: sinon.stub() + err:-> log: sinon.stub() @process = new events.EventEmitter @process.stdout = new events.EventEmitter @process.stderr = new events.EventEmitter + @child = spawn: sinon.stub().returns(@process) - exec: sinon.stub().callsArgWith(1, null, " 109042 2 files") + + @metrics = Timer: class Timer done: sinon.stub() @@ -30,6 +34,7 @@ describe "ArchiveManager", -> @source = "/path/to/zip/source.zip" @destination = "/path/to/zip/destination" @callback = sinon.stub() + @ArchiveManager._isZipTooLarge = sinon.stub().callsArgWith(1, null, false) describe "successfully", -> beforeEach (done) -> @@ -61,7 +66,7 @@ describe "ArchiveManager", -> describe "with a zip that is too large", -> beforeEach (done) -> - @child.exec = sinon.stub().callsArgWith(1, null, " 10000000000009042 2 files") + @ArchiveManager._isZipTooLarge = sinon.stub().callsArgWith(1, null, true) @ArchiveManager.extractZipArchive @source, @destination, (error) => @callback(error) done() @@ -85,6 +90,45 @@ describe "ArchiveManager", -> it "should log out the error", -> @logger.error.called.should.equal true + describe "_isZipTooLarge", -> + beforeEach -> + @output = (totalSize)->" Length Date Time Name \n-------- ---- ---- ---- \n241 03-12-16 12:20 main.tex \n108801 03-12-16 12:20 ddd/x1J5kHh.jpg \n-------- ------- \n#{totalSize} 2 files\n" + + it "should return false with small output", (done)-> + @ArchiveManager._isZipTooLarge @source, (error, isTooLarge) => + isTooLarge.should.equal false + done() + @process.stdout.emit "data", @output("109042") + @process.emit "exit" + + it "should return true with large bytes", (done)-> + @ArchiveManager._isZipTooLarge @source, (error, isTooLarge) => + isTooLarge.should.equal true + done() + @process.stdout.emit "data", @output("1090000000000000042") + @process.emit "exit" + + it "should return error on no data", (done)-> + @ArchiveManager._isZipTooLarge @source, (error, isTooLarge) => + expect(error).to.exist + done() + @process.stdout.emit "data", "" + @process.emit "exit" + + it "should return error if it didn't get a number", (done)-> + @ArchiveManager._isZipTooLarge @source, (error, isTooLarge) => + expect(error).to.exist + done() + @process.stdout.emit "data", @output("total_size_string") + @process.emit "exit" + + it "should return error if the is only a bit of data", (done)-> + @ArchiveManager._isZipTooLarge @source, (error, isTooLarge) => + expect(error).to.exist + done() + @process.stdout.emit "data", " Length Date Time Name \n--------" + @process.emit "exit" + describe "findTopLevelDirectory", -> beforeEach -> @fs.readdir = sinon.stub()