diff --git a/services/web/app/src/Features/Uploads/ArchiveManager.mjs b/services/web/app/src/Features/Uploads/ArchiveManager.mjs index 995c394744..a2ecb3b13f 100644 --- a/services/web/app/src/Features/Uploads/ArchiveManager.mjs +++ b/services/web/app/src/Features/Uploads/ArchiveManager.mjs @@ -1,24 +1,11 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import logger from '@overleaf/logger' - import OError from '@overleaf/o-error' import metrics from '@overleaf/metrics' import fs from 'node:fs' +import fsPromises from 'node:fs/promises' import Path from 'node:path' import { pipeline } from 'node:stream' +import { callbackify } from 'node:util' import yauzl from 'yauzl' import Settings from '@overleaf/settings' import { @@ -26,239 +13,200 @@ import { EmptyZipFileError, ZipContentsTooLargeError, } from './ArchiveErrors.mjs' -import _ from 'lodash' -import { promisify } from '@overleaf/promise-utils' const ONE_MEG = 1024 * 1024 -const ArchiveManager = { - _isZipTooLarge(source, callback) { - if (callback == null) { - callback = function () {} +/** + * Check if a zip entry's file path is safe and return the destination path. + * Returns null for directory entries. + * Throws for relative or unnormalized paths. + */ +function _checkFilePath(entry, destination) { + // transform backslashes to forwardslashes to accommodate badly-behaved zip archives + const transformedFilename = entry.fileName.replace(/\\/g, '/') + // check if the entry is a directory + if (/\/$/.test(transformedFilename)) { + return null + } + // check that the file does not use a relative path + for (const dir of transformedFilename.split('/')) { + if (dir === '..') { + throw new Error('relative path') } - callback = _.once(callback) + } + // check that the destination file path is normalized + const dest = `${destination}/${transformedFilename}` + if (dest !== Path.normalize(dest)) { + throw new Error('unnormalized path') + } + return dest +} - let totalSizeInBytes = null - return yauzl.open(source, { lazyEntries: true }, function (err, zipfile) { - if (err != null) { - return callback(new InvalidZipFileError().withCause(err)) +// Kept callback-based: called from event handler in _extractZipFiles +function _writeFileEntry(zipfile, entry, destFile, callback) { + fs.mkdir(Path.dirname(destFile), { recursive: true }, err => { + if (err) return callback(err) + zipfile.openReadStream(entry, (err, readStream) => { + if (err) return callback(err) + const writeStream = fs.createWriteStream(destFile) + pipeline(readStream, writeStream, callback) + }) + }) +} + +function _isZipTooLarge(source) { + return new Promise((resolve, reject) => { + let settled = false + const done = (err, result) => { + if (settled) return + settled = true + if (err) reject(err) + else resolve(result) + } + + yauzl.open(source, { lazyEntries: true }, (err, zipfile) => { + if (err) { + return done(new InvalidZipFileError().withCause(err)) } if ( Settings.maxEntitiesPerProject != null && zipfile.entryCount > Settings.maxEntitiesPerProject ) { - return callback(null, true) // too many files in zip file + zipfile.close() + return done(null, true) // too many files in zip file } - zipfile.on('error', callback) + let totalSizeInBytes = null + zipfile.on('error', err => done(err)) // read all the entries zipfile.readEntry() - zipfile.on('entry', function (entry) { + zipfile.on('entry', entry => { totalSizeInBytes += entry.uncompressedSize - return zipfile.readEntry() - }) // get the next entry + zipfile.readEntry() + }) // no more entries to read - return zipfile.on('end', function () { + zipfile.on('end', () => { if (totalSizeInBytes == null || isNaN(totalSizeInBytes)) { logger.warn( { source, totalSizeInBytes }, 'error getting bytes of zip' ) - return callback( - new InvalidZipFileError({ info: { totalSizeInBytes } }) - ) + return done(new InvalidZipFileError({ info: { totalSizeInBytes } })) } - const isTooLarge = totalSizeInBytes > ONE_MEG * 300 - return callback(null, isTooLarge) + done(null, totalSizeInBytes > ONE_MEG * 300) }) }) - }, + }) +} - _checkFilePath(entry, destination, callback) { - // transform backslashes to forwardslashes to accommodate badly-behaved zip archives - if (callback == null) { - callback = function () {} +function _extractZipFiles(source, destination) { + return new Promise((resolve, reject) => { + let settled = false + const done = err => { + if (settled) return + settled = true + if (err) reject(err) + else resolve() } - const transformedFilename = entry.fileName.replace(/\\/g, '/') - // check if the entry is a directory - const endsWithSlash = /\/$/ - if (endsWithSlash.test(transformedFilename)) { - return callback() // don't give a destfile for directory - } - // check that the file does not use a relative path - for (const dir of Array.from(transformedFilename.split('/'))) { - if (dir === '..') { - return callback(new Error('relative path')) - } - } - // check that the destination file path is normalized - const dest = `${destination}/${transformedFilename}` - if (dest !== Path.normalize(dest)) { - return callback(new Error('unnormalized path')) - } else { - return callback(null, dest) - } - }, - _writeFileEntry(zipfile, entry, destFile, callback) { - if (callback == null) { - callback = function () {} - } - callback = _.once(callback) + yauzl.open(source, { lazyEntries: true }, (err, zipfile) => { + if (err) return done(err) - fs.mkdir(Path.dirname(destFile), { recursive: true }, function (err) { - if (err != null) { - return callback(err) - } - zipfile.openReadStream(entry, function (err, readStream) { - if (err != null) { - return callback(err) - } - const writeStream = fs.createWriteStream(destFile) - pipeline(readStream, writeStream, callback) - }) - }) - }, - - _extractZipFiles(source, destination, callback) { - if (callback == null) { - callback = function () {} - } - callback = _.once(callback) - - return yauzl.open(source, { lazyEntries: true }, function (err, zipfile) { - if (err != null) { - return callback(err) - } - zipfile.on('error', callback) + zipfile.on('error', err => done(err)) // read all the entries zipfile.readEntry() let entryFileCount = 0 - zipfile.on('entry', function (entry) { - return ArchiveManager._checkFilePath( - entry, - destination, - function (err, destFile) { - if (err != null) { - logger.warn( - { err, source, destination }, - 'skipping bad file path' - ) - zipfile.readEntry() // bad path, just skip to the next file - return - } - if (destFile != null) { - // only write files - return ArchiveManager._writeFileEntry( - zipfile, - entry, + zipfile.on('entry', entry => { + let destFile + try { + destFile = _checkFilePath(entry, destination) + } catch (err) { + logger.warn({ err, source, destination }, 'skipping bad file path') + zipfile.readEntry() // bad path, just skip to the next file + return + } + if (destFile) { + // only write files + _writeFileEntry(zipfile, entry, destFile, err => { + if (err) { + OError.tag(err, 'error unzipping file entry', { + source, destFile, - function (err) { - if (err != null) { - OError.tag(err, 'error unzipping file entry', { - source, - destFile, - }) - zipfile.close() // bail out, stop reading file entries - return callback(err) - } else { - entryFileCount++ - return zipfile.readEntry() - } - } - ) // continue to the next file + }) + zipfile.close() // bail out, stop reading file entries + done(err) } else { - // if it's a directory, continue - return zipfile.readEntry() + entryFileCount++ + zipfile.readEntry() // continue to the next file } - } - ) - }) - // no more entries to read - return zipfile.on('end', () => { - if (entryFileCount > 0) { - callback() + }) } else { - callback(new EmptyZipFileError()) + // if it's a directory, continue + zipfile.readEntry() + } + }) + + // no more entries to read + zipfile.on('end', () => { + if (entryFileCount > 0) { + done() + } else { + done(new EmptyZipFileError()) } }) }) - }, + }) +} - extractZipArchive(source, destination, _callback) { - if (_callback == null) { - _callback = function () {} - } - const callback = function (...args) { - _callback(...Array.from(args || [])) - return (_callback = function () {}) +async function extractZipArchive(source, destination) { + let isTooLarge + try { + isTooLarge = await ArchiveManager._isZipTooLarge(source) + } catch (err) { + OError.tag(err, 'error checking size of zip file') + throw err + } + + if (isTooLarge) { + throw new ZipContentsTooLargeError() + } + + const timer = new metrics.Timer('unzipDirectory') + logger.debug({ source, destination }, 'unzipping file') + try { + await _extractZipFiles(source, destination) + } catch (err) { + OError.tag(err, 'unzip failed', { source, destination }) + throw err + } finally { + timer.done() + } +} + +async function findTopLevelDirectory(directory) { + const files = await fsPromises.readdir(directory) + if (files.length === 1) { + const childPath = Path.join(directory, files[0]) + const stat = await fsPromises.stat(childPath) + if (stat.isDirectory()) { + return childPath } + } + return directory +} - return ArchiveManager._isZipTooLarge(source, function (err, isTooLarge) { - if (err != null) { - OError.tag(err, 'error checking size of zip file') - return callback(err) - } - - if (isTooLarge) { - return callback(new ZipContentsTooLargeError()) - } - - const timer = new metrics.Timer('unzipDirectory') - logger.debug({ source, destination }, 'unzipping file') - - return ArchiveManager._extractZipFiles( - source, - destination, - function (err) { - timer.done() - if (err != null) { - OError.tag(err, 'unzip failed', { - source, - destination, - }) - return callback(err) - } else { - return callback() - } - } - ) - }) - }, - - findTopLevelDirectory(directory, callback) { - if (callback == null) { - callback = function () {} - } - return fs.readdir(directory, function (error, files) { - if (error != null) { - return callback(error) - } - if (files.length === 1) { - const childPath = Path.join(directory, files[0]) - return fs.stat(childPath, function (error, stat) { - if (error != null) { - return callback(error) - } - if (stat.isDirectory()) { - return callback(null, childPath) - } else { - return callback(null, directory) - } - }) - } else { - return callback(null, directory) - } - }) +const ArchiveManager = { + _isZipTooLarge, + extractZipArchive: callbackify(extractZipArchive), + findTopLevelDirectory: callbackify(findTopLevelDirectory), + promises: { + extractZipArchive, + findTopLevelDirectory, }, } -ArchiveManager.promises = { - extractZipArchive: promisify(ArchiveManager.extractZipArchive), - findTopLevelDirectory: promisify(ArchiveManager.findTopLevelDirectory), -} export default ArchiveManager diff --git a/services/web/test/unit/src/Uploads/ArchiveManager.test.mjs b/services/web/test/unit/src/Uploads/ArchiveManager.test.mjs index 67cc6ea798..ad57c170ea 100644 --- a/services/web/test/unit/src/Uploads/ArchiveManager.test.mjs +++ b/services/web/test/unit/src/Uploads/ArchiveManager.test.mjs @@ -1,12 +1,4 @@ import { vi, expect } from 'vitest' -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import sinon from 'sinon' import ArchiveErrors from '../../../../app/src/Features/Uploads/ArchiveErrors.mjs' import events from 'node:events' @@ -20,24 +12,17 @@ const modulePath = '../../../../app/src/Features/Uploads/ArchiveManager.mjs' describe('ArchiveManager', function () { beforeEach(async function (ctx) { - let Timer ctx.metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), + Timer: class Timer {}, } + ctx.metrics.Timer.prototype.done = sinon.stub() ctx.zipfile = new events.EventEmitter() ctx.zipfile.readEntry = sinon.stub() ctx.zipfile.close = sinon.stub() + ctx.Settings = {} vi.doMock('@overleaf/settings', () => ({ - default: {}, + default: ctx.Settings, })) vi.doMock('yauzl', () => ({ @@ -49,50 +34,57 @@ describe('ArchiveManager', function () { vi.doMock('@overleaf/metrics', () => ({ default: ctx.metrics, })) - ctx.fs = { mkdir: sinon.stub().yields(), stat: sinon.stub() } + ctx.fs = { mkdir: sinon.stub().yields() } vi.doMock('fs', () => ({ default: ctx.fs, })) + ctx.fsPromises = { + readdir: sinon.stub(), + stat: sinon.stub(), + } + vi.doMock('fs/promises', () => ({ + default: ctx.fsPromises, + })) + vi.doMock( '../../../../app/src/Features/Uploads/ArchiveErrors', () => ArchiveErrors ) ctx.ArchiveManager = (await import(modulePath)).default - ctx.callback = sinon.stub() }) describe('extractZipArchive', function () { beforeEach(function (ctx) { ctx.source = '/path/to/zip/source.zip' ctx.destination = '/path/to/zip/destination' - ctx.ArchiveManager._isZipTooLarge = sinon - .stub() - .callsArgWith(1, null, false) + ctx.ArchiveManager._isZipTooLarge = sinon.stub().resolves(false) }) describe('successfully', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.readStream = new PassThrough() - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, null, ctx.readStream) - ctx.writeStream = new PassThrough() - ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) - sinon.spy(ctx.writeStream, 'destroy') - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - resolve - ) + ctx.readStream = new PassThrough() + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, null, ctx.readStream) + ctx.writeStream = new PassThrough() + ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) + sinon.spy(ctx.writeStream, 'destroy') - // entry contains a single file - ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) - ctx.readStream.end() - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + // entry contains a single file + ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) + ctx.readStream.end() + // flush pipeline callback (fires via nextTick) + await new Promise(resolve => process.nextTick(resolve)) + ctx.zipfile.emit('end') + await promise }) it('should run yauzl', function (ctx) { @@ -106,107 +98,96 @@ describe('ArchiveManager', function () { describe('with a zipfile containing an empty directory', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.readStream = new PassThrough() - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, null, ctx.readStream) - ctx.writeStream = new PassThrough() - ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) - sinon.spy(ctx.writeStream, 'destroy') - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } - ) + ctx.readStream = new PassThrough() + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, null, ctx.readStream) + ctx.writeStream = new PassThrough() + ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) + sinon.spy(ctx.writeStream, 'destroy') - // entry contains a single, empty directory - ctx.zipfile.emit('entry', { fileName: 'testdir/' }) - ctx.readStream.end() - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + // entry contains a single, empty directory + ctx.zipfile.emit('entry', { fileName: 'testdir/' }) + ctx.readStream.end() + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - sinon.assert.calledWithExactly( - ctx.callback, - sinon.match.instanceOf(ArchiveErrors.EmptyZipFileError) - ) + it('should reject with an EmptyZipFileError', function (ctx) { + expect(ctx.error).to.be.instanceOf(ArchiveErrors.EmptyZipFileError) }) }) describe('with an empty zipfile', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } - ) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - sinon.assert.calledWithExactly( - ctx.callback, - sinon.match.instanceOf(ArchiveErrors.EmptyZipFileError) - ) + it('should reject with an EmptyZipFileError', function (ctx) { + expect(ctx.error).to.be.instanceOf(ArchiveErrors.EmptyZipFileError) }) }) describe('with an error in the zip file header', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.yauzl.open = sinon - .stub() - .callsArgWith(2, new ArchiveErrors.InvalidZipFileError()) - ctx.ArchiveManager.extractZipArchive( + ctx.yauzl.open = sinon + .stub() + .callsArgWith(2, new ArchiveErrors.InvalidZipFileError()) + + try { + await ctx.ArchiveManager.promises.extractZipArchive( ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } + ctx.destination ) - }) + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - sinon.assert.calledWithExactly( - ctx.callback, - sinon.match.instanceOf(ArchiveErrors.InvalidZipFileError) - ) + it('should reject with an error', function (ctx) { + expect(ctx.error).to.be.instanceOf(ArchiveErrors.InvalidZipFileError) }) }) describe('with a zip that is too large', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge = sinon - .stub() - .callsArgWith(1, null, true) - ctx.ArchiveManager.extractZipArchive( + ctx.ArchiveManager._isZipTooLarge = sinon.stub().resolves(true) + + try { + await ctx.ArchiveManager.promises.extractZipArchive( ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } + ctx.destination ) - }) + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - sinon.assert.calledWithExactly( - ctx.callback, - sinon.match.instanceOf(ArchiveErrors.ZipContentsTooLargeError) + it('should reject with a ZipContentsTooLargeError', function (ctx) { + expect(ctx.error).to.be.instanceOf( + ArchiveErrors.ZipContentsTooLargeError ) }) @@ -217,96 +198,101 @@ describe('ArchiveManager', function () { describe('with an error in the extracted files', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } - ) - ctx.zipfile.emit('error', new Error('Something went wrong')) - }) + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + ctx.zipfile.emit('error', new Error('Something went wrong')) + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - return ctx.callback.should.have.been.calledWithExactly( - sinon.match - .instanceOf(Error) - .and(sinon.match.has('message', 'Something went wrong')) - ) + it('should reject with an error', function (ctx) { + expect(ctx.error) + .to.be.instanceOf(Error) + .and.have.property('message', 'Something went wrong') }) }) describe('with a relative extracted file path', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.zipfile.openReadStream = sinon.stub() - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - return resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: '../testfile.txt' }) - return ctx.zipfile.emit('end') - }) + ctx.zipfile.openReadStream = sinon.stub() + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: '../testfile.txt' }) + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should not write try to read the file entry', function (ctx) { - return ctx.zipfile.openReadStream.called.should.equal(false) + it('should not try to read the file entry', function (ctx) { + ctx.zipfile.openReadStream.called.should.equal(false) }) }) describe('with an unnormalized extracted file path', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.zipfile.openReadStream = sinon.stub() - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - return resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'foo/./testfile.txt' }) - return ctx.zipfile.emit('end') - }) + ctx.zipfile.openReadStream = sinon.stub() + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'foo/./testfile.txt' }) + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) it('should not try to read the file entry', function (ctx) { - return ctx.zipfile.openReadStream.called.should.equal(false) + ctx.zipfile.openReadStream.called.should.equal(false) }) }) describe('with backslashes in the path', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.readStream = new PassThrough() - ctx.writeStream = new PassThrough() - ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) - sinon.spy(ctx.writeStream, 'destroy') - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, null, ctx.readStream) - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - return resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'wombat\\foo.tex' }) - ctx.readStream.end() - ctx.zipfile.emit('entry', { fileName: 'potato\\bar.tex' }) - ctx.readStream.end() - return ctx.zipfile.emit('end') - }) + ctx.readStream = new PassThrough() + ctx.writeStream = new PassThrough() + ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) + sinon.spy(ctx.writeStream, 'destroy') + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, null, ctx.readStream) + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'wombat\\foo.tex' }) + ctx.readStream.end() + ctx.zipfile.emit('entry', { fileName: 'potato\\bar.tex' }) + ctx.readStream.end() + ctx.zipfile.emit('end') + // Pipeline doesn't complete in this test (shared streams); + // we only verify method call arguments, so swallow the rejection. + await promise.catch(() => {}) }) it('should read the file entry with its original path', function (ctx) { @@ -339,19 +325,22 @@ describe('ArchiveManager', function () { describe('with a directory entry', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.zipfile.openReadStream = sinon.stub() - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'testdir/' }) - ctx.zipfile.emit('end') - }) + ctx.zipfile.openReadStream = sinon.stub() + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'testdir/' }) + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) it('should not try to read the entry', function (ctx) { @@ -361,30 +350,31 @@ describe('ArchiveManager', function () { describe('with an error opening the file read stream', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, new Error('Something went wrong')) - ctx.writeStream = new PassThrough() - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) - ctx.zipfile.emit('end') - }) + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, new Error('Something went wrong')) + ctx.writeStream = new PassThrough() + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) + ctx.zipfile.emit('end') + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - ctx.callback.should.have.been.calledWithExactly( - sinon.match - .instanceOf(Error) - .and(sinon.match.has('message', 'Something went wrong')) - ) + it('should reject with an error', function (ctx) { + expect(ctx.error) + .to.be.instanceOf(Error) + .and.have.property('message', 'Something went wrong') }) it('should close the zipfile', function (ctx) { @@ -394,33 +384,34 @@ describe('ArchiveManager', function () { describe('with an error in the file read stream', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.readStream = new PassThrough() - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, null, ctx.readStream) - ctx.writeStream = new PassThrough() - ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) - sinon.spy(ctx.writeStream, 'destroy') - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - return resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) - ctx.readStream.emit('error', new Error('Something went wrong')) - }) + ctx.readStream = new PassThrough() + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, null, ctx.readStream) + ctx.writeStream = new PassThrough() + ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) + sinon.spy(ctx.writeStream, 'destroy') + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) + ctx.readStream.emit('error', new Error('Something went wrong')) + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - ctx.callback.should.have.been.calledWithExactly( - sinon.match - .instanceOf(Error) - .and(sinon.match.has('message', 'Something went wrong')) - ) + it('should reject with an error', function (ctx) { + expect(ctx.error) + .to.be.instanceOf(Error) + .and.have.property('message', 'Something went wrong') }) it('should close the zipfile', function (ctx) { @@ -430,34 +421,35 @@ describe('ArchiveManager', function () { describe('with an error in the file write stream', function () { beforeEach(async function (ctx) { - await new Promise(resolve => { - ctx.readStream = new PassThrough() - sinon.spy(ctx.readStream, 'destroy') - ctx.zipfile.openReadStream = sinon - .stub() - .callsArgWith(1, null, ctx.readStream) - ctx.writeStream = new PassThrough() - ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) - sinon.spy(ctx.writeStream, 'destroy') - ctx.ArchiveManager.extractZipArchive( - ctx.source, - ctx.destination, - error => { - ctx.callback(error) - return resolve() - } - ) - ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) - ctx.writeStream.emit('error', new Error('Something went wrong')) - }) + ctx.readStream = new PassThrough() + sinon.spy(ctx.readStream, 'destroy') + ctx.zipfile.openReadStream = sinon + .stub() + .callsArgWith(1, null, ctx.readStream) + ctx.writeStream = new PassThrough() + ctx.fs.createWriteStream = sinon.stub().returns(ctx.writeStream) + sinon.spy(ctx.writeStream, 'destroy') + + const promise = ctx.ArchiveManager.promises.extractZipArchive( + ctx.source, + ctx.destination + ) + await Promise.resolve() + + ctx.zipfile.emit('entry', { fileName: 'testfile.txt' }) + ctx.writeStream.emit('error', new Error('Something went wrong')) + + try { + await promise + } catch (error) { + ctx.error = error + } }) - it('should return the callback with an error', function (ctx) { - ctx.callback.should.have.been.calledWithExactly( - sinon.match - .instanceOf(Error) - .and(sinon.match.has('message', 'Something went wrong')) - ) + it('should reject with an error', function (ctx) { + expect(ctx.error) + .to.be.instanceOf(Error) + .and.have.property('message', 'Something went wrong') }) it('should destroy the readstream', function (ctx) { ctx.readStream.destroy.called.should.equal(true) @@ -471,122 +463,123 @@ describe('ArchiveManager', function () { describe('_isZipTooLarge', function () { it('should return false with small output', async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge(ctx.source, (error, isTooLarge) => { - expect(error).not.to.exist - isTooLarge.should.equal(false) - resolve() - }) - ctx.zipfile.emit('entry', { uncompressedSize: 109042 }) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager._isZipTooLarge(ctx.source) + ctx.zipfile.emit('entry', { uncompressedSize: 109042 }) + ctx.zipfile.emit('end') + const isTooLarge = await promise + isTooLarge.should.equal(false) }) it('should return true with large bytes', async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge(ctx.source, (error, isTooLarge) => { - expect(error).not.to.exist - isTooLarge.should.equal(true) - resolve() - }) - ctx.zipfile.emit('entry', { uncompressedSize: 109e16 }) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager._isZipTooLarge(ctx.source) + ctx.zipfile.emit('entry', { uncompressedSize: 109e16 }) + ctx.zipfile.emit('end') + const isTooLarge = await promise + isTooLarge.should.equal(true) }) it('should return error on no data', async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge(ctx.source, (error, isTooLarge) => { - expect(error).to.exist - resolve() - }) - ctx.zipfile.emit('entry', {}) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager._isZipTooLarge(ctx.source) + ctx.zipfile.emit('entry', {}) + ctx.zipfile.emit('end') + await expect(promise).to.be.rejectedWith( + ArchiveErrors.InvalidZipFileError + ) }) it("should return error if it didn't get a number", async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge(ctx.source, (error, isTooLarge) => { - expect(error).to.exist - resolve() - }) - ctx.zipfile.emit('entry', { uncompressedSize: 'random-error' }) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager._isZipTooLarge(ctx.source) + ctx.zipfile.emit('entry', { uncompressedSize: 'random-error' }) + ctx.zipfile.emit('end') + await expect(promise).to.be.rejectedWith( + ArchiveErrors.InvalidZipFileError + ) }) it('should return error if there is no data', async function (ctx) { - await new Promise(resolve => { - ctx.ArchiveManager._isZipTooLarge(ctx.source, (error, isTooLarge) => { - expect(error).to.exist - resolve() - }) - ctx.zipfile.emit('end') - }) + const promise = ctx.ArchiveManager._isZipTooLarge(ctx.source) + ctx.zipfile.emit('end') + await expect(promise).to.be.rejectedWith( + ArchiveErrors.InvalidZipFileError + ) + }) + + it('should return true and close zipfile when entryCount exceeds maxEntitiesPerProject', async function (ctx) { + ctx.zipfile.entryCount = 100 + ctx.Settings.maxEntitiesPerProject = 50 + const isTooLarge = await ctx.ArchiveManager._isZipTooLarge(ctx.source) + isTooLarge.should.equal(true) + ctx.zipfile.close.called.should.equal(true) }) }) describe('findTopLevelDirectory', function () { beforeEach(function (ctx) { - ctx.fs.readdir = sinon.stub() - ctx.fs.stat((ctx.directory = 'test/directory')) + ctx.directory = 'test/directory' }) describe('with multiple files', function () { - beforeEach(function (ctx) { - ctx.fs.readdir.callsArgWith(1, null, ['multiple', 'files']) - ctx.ArchiveManager.findTopLevelDirectory(ctx.directory, ctx.callback) + beforeEach(async function (ctx) { + ctx.fsPromises.readdir.resolves(['multiple', 'files']) + ctx.result = await ctx.ArchiveManager.promises.findTopLevelDirectory( + ctx.directory + ) }) it('should find the files in the directory', function (ctx) { - ctx.fs.readdir.calledWith(ctx.directory).should.equal(true) + ctx.fsPromises.readdir.calledWith(ctx.directory).should.equal(true) }) it('should return the original directory', function (ctx) { - ctx.callback.calledWith(null, ctx.directory).should.equal(true) + ctx.result.should.equal(ctx.directory) }) }) describe('with a single file (not folder)', function () { - beforeEach(function (ctx) { - ctx.fs.readdir.callsArgWith(1, null, ['foo.tex']) - ctx.fs.stat.callsArgWith(1, null, { + beforeEach(async function (ctx) { + ctx.fsPromises.readdir.resolves(['foo.tex']) + ctx.fsPromises.stat.resolves({ isDirectory() { return false }, }) - ctx.ArchiveManager.findTopLevelDirectory(ctx.directory, ctx.callback) + ctx.result = await ctx.ArchiveManager.promises.findTopLevelDirectory( + ctx.directory + ) }) it('should check if the file is a directory', function (ctx) { - ctx.fs.stat.calledWith(ctx.directory + '/foo.tex').should.equal(true) + ctx.fsPromises.stat + .calledWith(ctx.directory + '/foo.tex') + .should.equal(true) }) it('should return the original directory', function (ctx) { - ctx.callback.calledWith(null, ctx.directory).should.equal(true) + ctx.result.should.equal(ctx.directory) }) }) describe('with a single top-level folder', function () { - beforeEach(function (ctx) { - ctx.fs.readdir.callsArgWith(1, null, ['folder']) - ctx.fs.stat.callsArgWith(1, null, { + beforeEach(async function (ctx) { + ctx.fsPromises.readdir.resolves(['folder']) + ctx.fsPromises.stat.resolves({ isDirectory() { return true }, }) - ctx.ArchiveManager.findTopLevelDirectory(ctx.directory, ctx.callback) + ctx.result = await ctx.ArchiveManager.promises.findTopLevelDirectory( + ctx.directory + ) }) it('should check if the file is a directory', function (ctx) { - ctx.fs.stat.calledWith(ctx.directory + '/folder').should.equal(true) + ctx.fsPromises.stat + .calledWith(ctx.directory + '/folder') + .should.equal(true) }) it('should return the child directory', function (ctx) { - ctx.callback - .calledWith(null, ctx.directory + '/folder') - .should.equal(true) + ctx.result.should.equal(ctx.directory + '/folder') }) }) })