diff --git a/services/clsi/app/js/OutputController.js b/services/clsi/app/js/OutputController.js index 13f3dce84c..a1518bd97a 100644 --- a/services/clsi/app/js/OutputController.js +++ b/services/clsi/app/js/OutputController.js @@ -8,14 +8,12 @@ async function createOutputZip(req, res) { user_id: userId, build_id: buildId, } = req.params - const files = Array.isArray(req.query.files) ? req.query.files : [] - logger.debug({ projectId, userId, buildId, files }, 'Will create zip file') + logger.debug({ projectId, userId, buildId }, 'Will create zip file') const archive = await OutputFileArchiveManager.archiveFilesForBuild( projectId, userId, - buildId, - files + buildId ) archive.on('error', err => { diff --git a/services/clsi/app/js/OutputFileArchiveManager.js b/services/clsi/app/js/OutputFileArchiveManager.js index b165d850df..fb81ae6c35 100644 --- a/services/clsi/app/js/OutputFileArchiveManager.js +++ b/services/clsi/app/js/OutputFileArchiveManager.js @@ -1,9 +1,8 @@ -let OutputFileArchiveManager const archiver = require('archiver') const OutputCacheManager = require('./OutputCacheManager') const OutputFileFinder = require('./OutputFileFinder') const Settings = require('@overleaf/settings') -const { open } = require('node:fs/promises') +const { open, realpath } = require('node:fs/promises') const path = require('path') const { NotFoundError } = require('./Errors') @@ -17,23 +16,38 @@ function getContentDir(projectId, userId) { return `${Settings.path.outputDir}/${subDir}/` } -module.exports = OutputFileArchiveManager = { - async archiveFilesForBuild(projectId, userId, build, files = []) { - const validFiles = await (files.length > 0 - ? this._getRequestedOutputFiles(projectId, userId, build, files) - : this._getAllOutputFiles(projectId, userId, build)) +/** + * Is the provided path a symlink? + * @param {string} path + * @return {Promise} + */ +async function isSymlink(path) { + try { + const realPath = await realpath(path) + return realPath !== path + } catch (error) { + if (error.code === 'ELOOP') { + return true + } + throw error + } +} + +module.exports = { + async archiveFilesForBuild(projectId, userId, build) { + const validFiles = await this._getAllOutputFiles(projectId, userId, build) const archive = archiver('zip') - const missingFiles = files.filter( - file => !validFiles.includes(path.basename(file)) - ) + const missingFiles = [] for (const file of validFiles) { try { - const fileHandle = await open(file, 'r') - const fileStream = fileHandle.createReadStream() - archive.append(fileStream, { name: path.basename(file) }) + if (!(await isSymlink(file))) { + const fileHandle = await open(file, 'r') + const fileStream = fileHandle.createReadStream() + archive.append(fileStream, { name: path.basename(file) }) + } } catch (error) { missingFiles.push(file) } @@ -59,9 +73,11 @@ module.exports = OutputFileArchiveManager = { `${contentDir}${OutputCacheManager.path(build, '.')}` ) - return outputFiles.map( - ({ path }) => `${contentDir}${OutputCacheManager.path(build, path)}` - ) + return outputFiles + .filter(({ path }) => path !== 'output.pdf') + .map( + ({ path }) => `${contentDir}${OutputCacheManager.path(build, path)}` + ) } catch (error) { if ( error.code === 'ENOENT' || @@ -73,16 +89,4 @@ module.exports = OutputFileArchiveManager = { throw error } }, - - async _getRequestedOutputFiles(projectId, userId, build, files) { - const outputFiles = await OutputFileArchiveManager._getAllOutputFiles( - projectId, - userId, - build - ) - - return files.filter(file => - outputFiles.some(outputFile => file === path.basename(outputFile)) - ) - }, } diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index adb1574e4a..f46b65d370 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -418,19 +418,6 @@ module.exports = CompileController = { const qs = {} - if (req.params.file === 'output.zip' && req.query?.files) { - /** - * The output.zip file is generated on the fly and allows either: - * - * 1. All files to be downloaded (no files query parameter) - * 2. A specific set of files to be downloaded (files query parameter) - * - * As the frontend separates the PDF download and ignores several other output - * files we will generally need to tell CLSI specifically what is required. - */ - qs.files = req.query.files - } - const url = CompileController._getFileUrl( projectId, userId, @@ -606,20 +593,10 @@ module.exports = CompileController = { return next(err) } url = new URL(`${Settings.apis.clsi.url}${url}`) - - const params = new URLSearchParams(persistenceOptions.qs) - - for (const [key, value] of Object.entries(qs)) { - if (Array.isArray(value)) { - for (const v of value) { - params.append(key, v) - } - continue - } - params.append(key, value) - } - - url.search = params.toString() + url.search = new URLSearchParams({ + ...persistenceOptions.qs, + ...qs, + }).toString() const timer = new Metrics.Timer( 'proxy_to_clsi', 1,