Răsfoiți Sursa

Remove `files` param from output.zip endpoint

GitOrigin-RevId: 300cd870578237ad8e61be4e9e41991f884aa754
andrew rumble 2 ani în urmă
părinte
comite
d627a0bee9

+ 2 - 4
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 => {

+ 32 - 28
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<boolean>}
+ */
+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))
-    )
-  },
 }

+ 4 - 27
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,