Prechádzať zdrojové kódy

[web] consolidate clsi downloads and add zod validation (#33069)

* [web] consolidate clsi downloads and add zod validation

* [validation-tools] make prettier happy

* [web] make clsiServerId optional

* [web] fix type of buildId

* [web] gracefully handle ObjectId

* [web] fix type of buildId

* [monorepo] address review feedback

- cjs export
- update module path in comments
- skip adding ?clsiserverid if not set
- allow nested output file download for submissions and add tests

* [web] address review feedback

* [web] cache one more zod schema

* [web] fix unit tests

GitOrigin-RevId: 0a1e618955983e035defd6d3c0528b81e0e85c95
Jakob Ackermann 3 mesiacov pred
rodič
commit
37cc65ec7e

+ 117 - 0
libraries/validation-tools/test/unit/src/zodHelpers.test.ts

@@ -217,4 +217,121 @@ describe('zodHelpers', () => {
       ])
     })
   })
+  describe('buildId', () => {
+    it('fails to parse when provided with an invalid buildId', () => {
+      const parsed = zz.buildId().safeParse('aa')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'invalid buildId',
+        }),
+      ])
+    })
+
+    it('parses successfully when provided with a valid buildId', () => {
+      const parsed = zz.buildId().safeParse('19d6c341530-878fff6cdab7fb0c')
+      expect(parsed.success).toBe(true)
+      expect(parsed.data).toBe('19d6c341530-878fff6cdab7fb0c')
+    })
+
+    it('fails to parse when provided with an editorBuildId', () => {
+      const parsed = zz
+        .buildId()
+        .safeParse(
+          '03b1d773-6203-4669-b365-6a0aa5625878-19d6c341530-878fff6cdab7fb0c'
+        )
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'invalid buildId',
+        }),
+      ])
+    })
+  })
+
+  describe('editorBuildId', () => {
+    it('fails to parse when provided with an invalid buildId', () => {
+      const parsed = zz.editorBuildId().safeParse('aa')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'invalid editorId-buildId',
+        }),
+      ])
+    })
+
+    it('fails to parse when provided with a buildId', () => {
+      const parsed = zz
+        .editorBuildId()
+        .safeParse('19d6c341530-878fff6cdab7fb0c')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'invalid editorId-buildId',
+        }),
+      ])
+    })
+
+    it('parses successfully when provided with a valid editorId-buildId', () => {
+      const parsed = zz
+        .editorBuildId()
+        .safeParse(
+          '03b1d773-6203-4669-b365-6a0aa5625878-19d6c341530-878fff6cdab7fb0c'
+        )
+      expect(parsed.success).toBe(true)
+      expect(parsed.data).toBe(
+        '03b1d773-6203-4669-b365-6a0aa5625878-19d6c341530-878fff6cdab7fb0c'
+      )
+    })
+  })
+  describe('filepath', () => {
+    it('fails to parse with empty input', () => {
+      const parsed = zz.filepath().safeParse('')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'path is empty',
+        }),
+      ])
+    })
+
+    it('fails to parse with absolute path', () => {
+      const parsed = zz.filepath().safeParse('/output.pdf')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'path is absolute',
+        }),
+      ])
+    })
+
+    it('fails to parse when provided with path traversal', () => {
+      const parsed = zz.filepath().safeParse('../output.pdf')
+      expect(parsed.success).toBe(false)
+      expect(parsed.error?.issues).toHaveLength(1)
+      expect(parsed.error?.issues).toMatchObject([
+        expect.objectContaining({
+          message: 'path traversal detected',
+        }),
+      ])
+    })
+
+    it('parses successfully when provided a valid path', () => {
+      const parsed = zz.filepath().safeParse('output.pdf')
+      expect(parsed.success).toBe(true)
+      expect(parsed.data).toBe('output.pdf')
+    })
+
+    it('parses successfully when provided a valid nested path', () => {
+      const parsed = zz.filepath().safeParse('foo/output.pdf')
+      expect(parsed.success).toBe(true)
+      expect(parsed.data).toBe('foo/output.pdf')
+    })
+  })
 })

+ 8 - 0
libraries/validation-tools/testUtils.js

@@ -0,0 +1,8 @@
+function asZodError(...def) {
+  return {
+    name: 'ZodError',
+    _zod: { def },
+  }
+}
+
+module.exports = { asZodError }

+ 25 - 0
libraries/validation-tools/zodHelpers.js

@@ -33,6 +33,31 @@ const zz = {
   datetimeNullable: options => datetimeSchema({ ...options, allowNull: true }),
   datetimeNullish: options =>
     datetimeSchema({ ...options, allowNull: true, allowUndefined: true }),
+  buildId: () =>
+    z.string().regex(/^[0-9a-f]+-[0-9a-f]+$/, { message: 'invalid buildId' }),
+  editorBuildId: () =>
+    z.string().regex(/^[a-f0-9-]{36}-[0-9a-f]+-[0-9a-f]+$/, {
+      message: 'invalid editorId-buildId',
+    }),
+  clsiServerId: () =>
+    z.string().regex(/^[a-z0-9-]+$/, { message: 'invalid clsiServerId' }),
+  compileBackendClass: () =>
+    z
+      .string()
+      .regex(/^[a-z0-9-]+$/, { message: 'invalid compileBackendClass' }),
+  compileGroup: () =>
+    z.enum(['alpha', 'gvisor', 'standard', 'priority'], {
+      message: 'invalid compileGroup',
+    }),
+  submissionId: () => z.string().regex(/^[a-zA-Z0-9_-]+$/),
+  filepath: () =>
+    z
+      .string()
+      .nonempty({ message: 'path is empty' })
+      .refine(s => !s.startsWith('/'), { message: 'path is absolute' })
+      .refine(s => !s.split('/').includes('..'), {
+        message: 'path traversal detected',
+      }),
 }
 
 module.exports = { zz }

+ 6 - 6
services/clsi/test/unit/js/CompileController.test.js

@@ -82,12 +82,12 @@ describe('CompileController', () => {
           path: 'output.pdf',
           type: 'pdf',
           size: 1337,
-          build: 1234,
+          build: '1234-5678',
         },
         {
           path: 'output.log',
           type: 'log',
-          build: 1234,
+          build: '1234-5678',
         },
       ]
       ctx.RequestParser.parse = sinon.stub().callsArgWith(1, null, ctx.request)
@@ -190,12 +190,12 @@ describe('CompileController', () => {
           {
             path: 'fake_output.pdf',
             type: 'pdf',
-            build: 1234,
+            build: '1234-5678',
           },
           {
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: '1234-5678',
           },
         ]
         ctx.CompileManager.doCompileWithLock = sinon
@@ -239,12 +239,12 @@ describe('CompileController', () => {
             path: 'output.pdf',
             type: 'pdf',
             size: 0,
-            build: 1234,
+            build: '1234-5678',
           },
           {
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: '1234-5678',
           },
         ]
         ctx.CompileManager.doCompileWithLock = sinon

+ 2 - 2
services/clsi/test/unit/js/CompileManager.test.js

@@ -27,12 +27,12 @@ describe('CompileManager', () => {
       {
         path: 'output.log',
         type: 'log',
-        build: 1234,
+        build: '1234-5678',
       },
       {
         path: 'output.pdf',
         type: 'pdf',
-        build: 1234,
+        build: '1234-5678',
       },
     ]
     ctx.buildId = '00000000000-0000000000000000'

+ 19 - 7
services/web/app/src/Features/Compile/ClsiCacheController.mjs

@@ -13,6 +13,18 @@ import ClsiCacheHandler from './ClsiCacheHandler.mjs'
 import ProjectGetter from '../Project/ProjectGetter.mjs'
 import { MeteredStream } from '@overleaf/stream-utils'
 import Metrics from '@overleaf/metrics'
+import { z, zz } from '@overleaf/validation-tools'
+import { parseReq } from '../../infrastructure/Validation.mjs'
+
+const downloadFromCacheSchema = z.object({
+  params: z.object({
+    Project_id: zz.objectId(),
+    editorBuildId: zz.editorBuildId(),
+    filename: zz.filepath().refine(s => ClsiCacheHandler.isAllowedFilename(s), {
+      message: 'path is not allowed',
+    }),
+  }),
+})
 
 /**
  * Download a file from a specific build on the clsi-cache.
@@ -22,12 +34,14 @@ import Metrics from '@overleaf/metrics'
  * @return {Promise<*>}
  */
 async function downloadFromCache(req, res) {
-  const { Project_id: projectId, buildId, filename } = req.params
+  const {
+    params: { Project_id: projectId, editorBuildId, filename },
+  } = parseReq(req, downloadFromCacheSchema)
   return await _downloadFromCacheWithParams(
     req,
     res,
     projectId,
-    buildId,
+    editorBuildId,
     filename
   )
 }
@@ -38,18 +52,16 @@ async function downloadFromCache(req, res) {
  * @param req
  * @param res
  * @param projectId
- * @param buildId
+ * @param editorBuildId
  * @param filename
  * @param projectId
- * @param buildId
- * @param filename
  * @return {Promise<*>}
  */
 async function _downloadFromCacheWithParams(
   req,
   res,
   projectId,
-  buildId,
+  editorBuildId,
   filename
 ) {
   const userId = CompileController._getUserIdForCompile(req)
@@ -61,7 +73,7 @@ async function _downloadFromCacheWithParams(
       ClsiCacheHandler.getOutputFile(
         projectId,
         userId,
-        buildId,
+        editorBuildId,
         filename,
         ac.signal
       ),

+ 35 - 21
services/web/app/src/Features/Compile/ClsiCacheHandler.mjs

@@ -10,6 +10,7 @@ import OError from '@overleaf/o-error'
 import { NotFoundError, InvalidNameError } from '../Errors/Errors.js'
 import Features from '../../infrastructure/Features.mjs'
 import Path from 'node:path'
+import { zz } from '@overleaf/validation-tools'
 
 const TIMEOUT = 4_000
 
@@ -18,6 +19,29 @@ const TIMEOUT = 4_000
  */
 const lastFailures = new Map()
 
+/**
+ * Keep in sync with isAllowedFilename in services/clsi-cache/app/js/utils.js
+ *
+ * @param {string} filename
+ * @return {boolean}
+ */
+function isAllowedFilename(filename) {
+  return (
+    [
+      'output.blg',
+      'output.log',
+      'output.pdf',
+      'output.synctex.gz',
+      'output.overleaf.json',
+      'output.tar.gz',
+      // Not in web: 'history-resync.json.gz' is only read/written by clsi.
+      // The user/frontend should not be able to download it directly.
+      // We need to block access to it on the web layer.
+      // If we ever remove blockRestrictedUserFromProject from the history endpoints, we can remove this restriction.
+    ].includes(filename) || filename.endsWith('.blg')
+  )
+}
+
 /**
  * Keep in sync with validateFilename in services/clsi-cache/app/js/utils.js
  *
@@ -27,18 +51,7 @@ function validateFilename(filename) {
   if (filename.split('/').includes('..')) {
     throw new InvalidNameError('path traversal')
   }
-  if (
-    !(
-      [
-        'output.blg',
-        'output.log',
-        'output.pdf',
-        'output.synctex.gz',
-        'output.overleaf.json',
-        'output.tar.gz',
-      ].includes(filename) || filename.endsWith('.blg')
-    )
-  ) {
+  if (!isAllowedFilename(filename)) {
     throw new InvalidNameError('bad filename')
   }
 }
@@ -90,12 +103,14 @@ async function clearCache(projectId, userId) {
   )
 }
 
+const editorBuildIdSchema = zz.editorBuildId()
+
 /**
  * Get an output file from a specific build.
  *
  * @param projectId
  * @param userId
- * @param buildId
+ * @param editorBuildId
  * @param filename
  * @param signal
  * @return {Promise<{size: number, zone: string, shard: string, location: string, lastModified: Date, allFiles: string[]}>}
@@ -103,20 +118,18 @@ async function clearCache(projectId, userId) {
 async function getOutputFile(
   projectId,
   userId,
-  buildId,
+  editorBuildId,
   filename,
   signal = AbortSignal.timeout(TIMEOUT)
 ) {
   validateFilename(filename)
-  if (!/^[a-f0-9-]+$/.test(buildId)) {
-    throw new InvalidNameError('bad buildId')
-  }
+  editorBuildId = editorBuildIdSchema.parse(editorBuildId)
 
   let path = `/project/${projectId}`
   if (userId) {
     path += `/user/${userId}`
   }
-  path += `/build/${buildId}/search/output/${filename}`
+  path += `/build/${editorBuildId}/search/output/${filename}`
   return getRedirectWithFallback(projectId, userId, path, signal)
 }
 
@@ -269,7 +282,7 @@ async function prepareCacheSource(
  *
  * @param clsiCacheShard
  * @param submissionId
- * @param buildId
+ * @param editorBuildId
  * @param templateVersionId
  * @param imageName
  * @return {Promise<void>}
@@ -277,13 +290,13 @@ async function prepareCacheSource(
 async function exportSubmissionAsTemplate(
   clsiCacheShard,
   submissionId,
-  buildId,
+  editorBuildId,
   templateVersionId,
   imageName
 ) {
   imageName = Path.basename(imageName)
   const url = new URL(
-    `/submission/${submissionId}/build/${buildId}/export-as-template`,
+    `/submission/${submissionId}/build/${editorBuildId}/export-as-template`,
     Settings.apis.clsiCache.instances.find(i => i.shard === clsiCacheShard).url
   )
   try {
@@ -306,6 +319,7 @@ async function exportSubmissionAsTemplate(
 
 export default {
   TIMEOUT,
+  isAllowedFilename,
   getEgressLabel,
   clearCache,
   getOutputFile,

+ 8 - 7
services/web/app/src/Features/Compile/ClsiManager.mjs

@@ -24,6 +24,7 @@ import HistoryManager from '../History/HistoryManager.mjs'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
 import AnalyticsManager from '../Analytics/AnalyticsManager.mjs'
 import RedisWrapper from '../../infrastructure/RedisWrapper.mjs'
+import { getOutputFileURL } from './ClsiURLHelpers.mjs'
 
 // use the redis db with eviction policy enabled
 const rclient = RedisWrapper.client('clsi_cookie')
@@ -850,17 +851,17 @@ async function getContentFromDocUpdaterIfMatch(projectId, project, options) {
 async function getOutputFileStream(
   projectId,
   userId,
-  options,
   clsiServerId,
   buildId,
   outputFilePath
 ) {
-  const { compileBackendClass, compileGroup } = options
-  const url = new URL(Settings.apis.clsi.downloadHost)
-  url.pathname = `/project/${projectId}/user/${userId}/build/${buildId}/output/${outputFilePath}`
-  url.searchParams.set('compileBackendClass', compileBackendClass)
-  url.searchParams.set('compileGroup', compileGroup)
-  url.searchParams.set('clsiserverid', clsiServerId)
+  const url = getOutputFileURL(
+    projectId,
+    userId,
+    buildId,
+    outputFilePath,
+    clsiServerId
+  )
   try {
     const stream = await fetchStream(url, {
       signal: AbortSignal.timeout(OUTPUT_FILE_TIMEOUT_MS),

+ 85 - 0
services/web/app/src/Features/Compile/ClsiURLHelpers.mjs

@@ -0,0 +1,85 @@
+import { zz } from '@overleaf/validation-tools'
+import Settings from '@overleaf/settings'
+
+// Build zod schema once and use it below.
+const schema = {
+  compileBackendClass: zz.compileBackendClass(),
+  optionalClsiServerId: zz.clsiServerId().optional(),
+  projectIdOrSubmissionId: zz.objectId().or(zz.submissionId()),
+  optionalUserId: zz.objectId().optional(),
+  buildId: zz.buildId(),
+  filepath: zz.filepath(),
+}
+
+/**
+ * @param {string} projectIdOrSubmissionId
+ * @param {string|null} userId
+ * @param {string} buildId
+ * @param {string} compileBackendClass
+ * @param {string} clsiServerId
+ * @return {URL}
+ */
+export function getOutputZipURL(
+  projectIdOrSubmissionId,
+  userId,
+  buildId,
+  compileBackendClass,
+  clsiServerId
+) {
+  compileBackendClass = schema.compileBackendClass.parse(compileBackendClass)
+  clsiServerId = schema.optionalClsiServerId.parse(clsiServerId)
+  const url = new URL(Settings.apis.clsi.url)
+  url.pathname = getFilePath(
+    projectIdOrSubmissionId,
+    userId,
+    buildId,
+    'output.zip'
+  )
+  url.searchParams.set('compileBackendClass', compileBackendClass)
+  if (clsiServerId) url.searchParams.set('clsiserverid', clsiServerId)
+  return url
+}
+
+/**
+ * @param {string} projectIdOrSubmissionId
+ * @param {string|null} userId
+ * @param {string} buildId
+ * @param {string} file
+ * @param {string} clsiServerId
+ * @return {URL}
+ */
+export function getOutputFileURL(
+  projectIdOrSubmissionId,
+  userId,
+  buildId,
+  file,
+  clsiServerId
+) {
+  clsiServerId = schema.optionalClsiServerId.parse(clsiServerId)
+  const url = new URL(Settings.apis.clsi.downloadHost)
+  url.pathname = getFilePath(projectIdOrSubmissionId, userId, buildId, file)
+  if (clsiServerId) url.searchParams.set('clsiserverid', clsiServerId)
+  return url
+}
+
+/**
+ * @param {string} projectIdOrSubmissionId
+ * @param {string|null} userId
+ * @param {string} buildId
+ * @param {string} file
+ * @return {string}
+ */
+export function getFilePath(projectIdOrSubmissionId, userId, buildId, file) {
+  projectIdOrSubmissionId = schema.projectIdOrSubmissionId.parse(
+    projectIdOrSubmissionId.toString()
+  )
+  userId = schema.optionalUserId.parse(userId?.toString())
+  buildId = schema.buildId.parse(buildId)
+  file = schema.filepath.parse(file)
+  let path = `/project/${projectIdOrSubmissionId}`
+  if (userId) {
+    path += `/user/${userId}`
+  }
+  path += `/build/${buildId}/output/${file}`
+  return path
+}

+ 300 - 315
services/web/app/src/Features/Compile/CompileController.mjs

@@ -1,7 +1,5 @@
 import { URL } from 'node:url'
 import { pipeline } from 'node:stream/promises'
-import { Cookie } from 'tough-cookie'
-import OError from '@overleaf/o-error'
 import Metrics from '@overleaf/metrics'
 import ProjectGetter from '../Project/ProjectGetter.mjs'
 import CompileManager from './CompileManager.mjs'
@@ -12,7 +10,6 @@ import Errors from '../Errors/Errors.js'
 import SessionManager from '../Authentication/SessionManager.mjs'
 import { RateLimiter } from '../../infrastructure/RateLimiter.mjs'
 import Validation from '../../infrastructure/Validation.mjs'
-import ClsiCookieManagerFactory from './ClsiCookieManager.mjs'
 import Path from 'node:path'
 import AnalyticsManager from '../Analytics/AnalyticsManager.mjs'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
@@ -24,16 +21,17 @@ import {
 import Features from '../../infrastructure/Features.mjs'
 import ClsiCacheController from './ClsiCacheController.mjs'
 import { prepareZipAttachment } from '../../infrastructure/Response.mjs'
+import ClsiCacheHandler from './ClsiCacheHandler.mjs'
+import {
+  getFilePath,
+  getOutputFileURL,
+  getOutputZipURL,
+} from './ClsiURLHelpers.mjs'
 
 const { z, zz, parseReq } = Validation
-const ClsiCookieManager = ClsiCookieManagerFactory(
-  Settings.apis.clsi?.backendGroupName
-)
 
 const COMPILE_TIMEOUT_MS = 12 * 60 * 1000
 
-const buildIdSchema = z.string().regex(/[a-z0-9-]/)
-
 const pdfDownloadRateLimiter = new RateLimiter('full-pdf-download', {
   points: 1000,
   duration: 60 * 60,
@@ -43,7 +41,7 @@ function getOutputFilesArchiveSpecification(projectId, userId, buildId) {
   const fileName = 'output.zip'
   return {
     path: fileName,
-    url: _CompileController._getFileUrl(projectId, userId, buildId, fileName),
+    url: getFilePath(projectId, userId, buildId, fileName),
     type: 'zip',
   }
 }
@@ -119,7 +117,7 @@ const deleteAuxFilesSchema = z.object({
     Project_id: zz.objectId(),
   }),
   query: z.object({
-    clsiserverid: z.string().optional(),
+    clsiserverid: zz.clsiServerId().optional(),
   }),
 })
 
@@ -128,11 +126,55 @@ const wordCountSchema = z.object({
     Project_id: zz.objectId(),
   }),
   query: z.object({
-    clsiserverid: z.string().optional(),
+    clsiserverid: zz.clsiServerId().optional(),
     file: z.string().optional(),
   }),
 })
 
+const getFileForSubmissionFromClsiSchema = z.object({
+  params: z.object({
+    submissionId: zz.submissionId(),
+    build_id: zz.buildId(),
+    file: zz.filepath(),
+  }),
+  query: z.object({
+    clsiserverid: zz.clsiServerId().optional(),
+  }),
+})
+
+const getFileFromClsiSchema = z.object({
+  params: z.object({
+    Project_id: zz.objectId(),
+    build_id: zz.buildId(),
+    file: zz.filepath(),
+  }),
+  query: z.object({
+    clsiserverid: zz.clsiServerId().optional(),
+    editorId: z.uuid().optional(),
+  }),
+})
+
+const getOutputPDFFromClsiSchema = z.object({
+  params: z.object({
+    Project_id: zz.objectId(),
+    build_id: zz.buildId(),
+  }),
+  query: z.object({
+    clsiserverid: zz.clsiServerId().optional(),
+    editorId: z.uuid().optional(),
+  }),
+})
+
+const getOutputZipFromClsiSchema = z.object({
+  params: z.object({
+    Project_id: zz.objectId(),
+    build_id: zz.buildId(),
+  }),
+  query: z.object({
+    clsiserverid: zz.clsiServerId().optional(),
+  }),
+})
+
 const _CompileController = {
   async compile(req, res) {
     res.setTimeout(COMPILE_TIMEOUT_MS)
@@ -331,18 +373,23 @@ const _CompileController = {
   },
 
   async downloadPdf(req, res) {
+    const {
+      params: { Project_id: projectId, build_id: buildId },
+      query: { clsiserverid: clsiServerId, editorId },
+    } = parseReq(req, getOutputPDFFromClsiSchema)
     Metrics.inc('pdf-downloads')
-    const projectId = req.params.Project_id
-    const rateLimit = () =>
-      pdfDownloadRateLimiter
-        .consume(req.ip, 1, { method: 'ip' })
-        .then(() => true)
-        .catch(err => {
-          if (err instanceof Error) {
-            throw err
-          }
-          return false
-        })
+    try {
+      await pdfDownloadRateLimiter.consume(req.ip, 1, { method: 'ip' })
+    } catch (err) {
+      if (err instanceof Error) {
+        logger.err({ err }, 'error checking rate limit for pdf download')
+        res.status(500).end()
+        return
+      }
+      logger.debug({ projectId, ip: req.ip }, 'rate limit hit downloading pdf')
+      res.status(429).end()
+      return
+    }
 
     const project = await ProjectGetter.promises.getProject(projectId, {
       name: 1,
@@ -357,38 +404,18 @@ const _CompileController = {
       res.setContentDisposition('inline', { filename })
     }
 
-    let canContinue
-    try {
-      canContinue = await rateLimit()
-    } catch (err) {
-      logger.err({ err }, 'error checking rate limit for pdf download')
-      res.sendStatus(500)
-      return
-    }
-
-    if (!canContinue) {
-      logger.debug({ projectId, ip: req.ip }, 'rate limit hit downloading pdf')
-      res.sendStatus(500) // should it be 429?
-    } else {
-      const userId = CompileController._getUserIdForCompile(req)
-
-      const url = _CompileController._getFileUrl(
-        projectId,
-        userId,
-        req.params.build_id,
-        'output.pdf'
-      )
-      // Align params with the generic output file download (via getFileFromClsi / getFileFromClsiWithoutUser).
-      req.params.file = 'output.pdf'
-      await CompileController._proxyToClsi(
-        projectId,
-        'output-file',
-        url,
-        {},
-        req,
-        res
-      )
-    }
+    const userId = CompileController._getUserIdForCompile(req)
+    await _downloadFromClsiNginx(
+      projectId,
+      userId,
+      editorId,
+      buildId,
+      'output.pdf',
+      clsiServerId,
+      'output-file',
+      req,
+      res
+    )
   },
 
   // Keep in sync with the logic for zip files in ProjectDownloadsController
@@ -397,10 +424,11 @@ const _CompileController = {
   },
 
   async deleteAuxFiles(req, res) {
-    const { params, query } = parseReq(req, deleteAuxFilesSchema)
-    const projectId = params.Project_id
-    const { clsiserverid } = query
-    const userId = await CompileController._getUserIdForCompile(req)
+    const {
+      params: { Project_id: projectId },
+      query: { clsiserverid },
+    } = parseReq(req, deleteAuxFilesSchema)
+    const userId = CompileController._getUserIdForCompile(req)
     await CompileManager.promises.deleteAuxFiles(
       projectId,
       userId,
@@ -413,9 +441,9 @@ const _CompileController = {
   async compileAndDownloadPdf(req, res) {
     const projectId = req.params.project_id
 
-    let outputFiles
+    let outputFiles, clsiServerId, buildId
     try {
-      ;({ outputFiles } = await CompileManager.promises
+      ;({ outputFiles, clsiServerId, buildId } = await CompileManager.promises
         // pass userId as null, since templates are an "anonymous" compile
         .compile(projectId, null, {}))
     } catch (err) {
@@ -435,19 +463,25 @@ const _CompileController = {
       res.sendStatus(500)
       return
     }
-    await CompileController._proxyToClsi(
+    await _downloadFromClsiNginx(
       projectId,
+      null,
+      null,
+      buildId,
+      'output.pdf',
+      clsiServerId,
       'output-file',
-      pdf.url,
-      {},
       req,
       res
     )
   },
 
   async getOutputZipFromClsi(req, res) {
-    const projectId = req.params.Project_id
     const userId = CompileController._getUserIdForCompile(req)
+    const {
+      params: { Project_id: projectId, build_id: buildId },
+      query: { clsiserverid: clsiServerId },
+    } = parseReq(req, getOutputZipFromClsiSchema)
 
     const project = await ProjectGetter.promises.getProject(projectId, {
       name: 1,
@@ -455,87 +489,57 @@ const _CompileController = {
     const filename = `${_CompileController._getSafeProjectName(project)}-output.zip`
     prepareZipAttachment(res, filename)
 
-    const qs = {}
-    const url = _CompileController._getFileUrl(
+    await _downloadFromClsi(
       projectId,
       userId,
-      req.params.build_id,
-      'output.zip'
-    )
-    await CompileController._proxyToClsi(
-      projectId,
+      null,
+      buildId,
+      'output.zip',
+      clsiServerId,
       'output-zip-file',
-      url,
-      qs,
       req,
       res
     )
   },
 
   async getFileFromClsi(req, res) {
-    const projectId = req.params.Project_id
     const userId = CompileController._getUserIdForCompile(req)
+    const {
+      params: { Project_id: projectId, build_id: buildId, file },
+      query: { clsiserverid: clsiServerId, editorId },
+    } = parseReq(req, getFileFromClsiSchema)
 
-    const qs = {}
-
-    const url = _CompileController._getFileUrl(
+    await _downloadFromClsiNginx(
       projectId,
       userId,
-      req.params.build_id,
-      req.params.file
-    )
-    await CompileController._proxyToClsi(
-      projectId,
+      editorId,
+      buildId,
+      file,
+      clsiServerId,
       'output-file',
-      url,
-      qs,
       req,
       res
     )
   },
 
-  async getFileFromClsiWithoutUser(req, res) {
-    const submissionId = req.params.submission_id
-    const url = _CompileController._getFileUrl(
+  async getFileForSubmissionFromClsi(req, res) {
+    const {
+      params: { submissionId, build_id: buildId, file },
+      query: { clsiserverid: clsiServerId },
+    } = parseReq(req, getFileForSubmissionFromClsiSchema)
+    await _downloadFromClsiNginx(
       submissionId,
       null,
-      req.params.build_id,
-      req.params.file
-    )
-    const limits = {
-      compileGroup:
-        req.body?.compileGroup ||
-        req.query?.compileGroup ||
-        Settings.defaultFeatures.compileGroup,
-      compileBackendClass: Settings.apis.clsi.submissionBackendClass,
-    }
-    await CompileController._proxyToClsiWithLimits(
-      submissionId,
+      null,
+      buildId,
+      file,
+      clsiServerId,
       'output-file',
-      url,
-      {},
-      limits,
       req,
       res
     )
   },
 
-  // compute a GET file url for a given project, user (optional), build (optional) and file
-  _getFileUrl(projectId, userId, buildId, file) {
-    let url
-    if (userId != null && buildId != null) {
-      url = `/project/${projectId}/user/${userId}/build/${buildId}/output/${file}`
-    } else if (userId != null) {
-      url = `/project/${projectId}/user/${userId}/output/${file}`
-    } else if (buildId != null) {
-      buildId = buildIdSchema.parse(buildId)
-      url = `/project/${projectId}/build/${buildId}/output/${file}`
-    } else {
-      url = `/project/${projectId}/output/${file}`
-    }
-    return url
-  },
-
   async proxySyncPdf(req, res) {
     const { page, h, v } = req.query
     if (!page?.match(/^\d+$/)) {
@@ -573,163 +577,6 @@ const _CompileController = {
     await _syncTeX(req, res, 'code', { file, line, column })
   },
 
-  async _proxyToClsi(projectId, action, url, qs, req, res) {
-    const limits =
-      await CompileManager.promises.getProjectCompileLimits(projectId)
-    return CompileController._proxyToClsiWithLimits(
-      projectId,
-      action,
-      url,
-      qs,
-      limits,
-      req,
-      res
-    )
-  },
-
-  async _proxyToClsiWithLimits(
-    projectId,
-    action,
-    requestPath,
-    qs,
-    limits,
-    req,
-    res
-  ) {
-    const persistenceOptions = await _getPersistenceOptions(
-      req,
-      projectId,
-      limits.compileGroup,
-      limits.compileBackendClass
-    ).catch(err => {
-      OError.tag(err, 'error getting cookie jar for clsi request')
-      throw err
-    })
-
-    const url = new URL(
-      action === 'output-zip-file'
-        ? Settings.apis.clsi.url
-        : Settings.apis.clsi.downloadHost
-    )
-    url.pathname = requestPath
-
-    const searchParams = {
-      ...persistenceOptions.qs,
-      ...qs,
-    }
-    for (const [key, value] of Object.entries(searchParams)) {
-      if (value !== undefined) {
-        // avoid sending "undefined" as a string value
-        url.searchParams.set(key, value)
-      }
-    }
-
-    const timer = new Metrics.Timer(
-      'proxy_to_clsi',
-      1,
-      { path: action },
-      [0, 100, 1000, 2000, 5000, 10000, 15000, 20000, 30000, 45000, 60000]
-    )
-    Metrics.inc('proxy_to_clsi', 1, { path: action, status: 'start' })
-    const ac = new AbortController()
-    let timeout = setTimeout(() => ac.abort(), 10_000)
-    try {
-      const { stream, response } = await fetchStreamWithResponse(url.href, {
-        method: req.method,
-        signal: ac.signal,
-        headers: persistenceOptions.headers,
-      })
-      if (req.destroyed) {
-        // The client has disconnected already, avoid trying to write into the broken connection.
-        Metrics.inc('proxy_to_clsi', 1, {
-          path: action,
-          status: 'req-aborted',
-        })
-        stream.destroy(new Error('user aborted the request'))
-        return
-      }
-      Metrics.inc('proxy_to_clsi', 1, {
-        path: action,
-        status: response.status,
-      })
-
-      for (const key of ['Content-Length', 'Content-Type']) {
-        if (response.headers.has(key)) {
-          res.setHeader(key, response.headers.get(key))
-        }
-      }
-
-      // Downloads can take a while on a slow connection, increase timeouts to 10min
-      const TEN_MINUTES_IN_MS = 10 * 60 * 1000
-      res.setTimeout(TEN_MINUTES_IN_MS)
-      clearTimeout(timeout)
-      timeout = setTimeout(() => ac.abort(), TEN_MINUTES_IN_MS)
-
-      // Disable buffering in nginx
-      res.setHeader('X-Accel-Buffering', 'no')
-
-      res.writeHead(response.status)
-      await pipeline(stream, res)
-      timer.labels.status = 'success'
-      timer.done()
-    } catch (err) {
-      if (canTryClsiCacheFallback(req, res, action, err)) {
-        await ClsiCacheController._downloadFromCacheWithParams(
-          req,
-          res,
-          projectId,
-          `${req.query.editorId}-${req.params.build_id}`,
-          req.params.file
-        )
-        return
-      }
-      const reqAborted = Boolean(req.destroyed)
-      const status = reqAborted ? 'req-aborted-late' : 'error'
-      timer.labels.status = status
-      const duration = timer.done()
-      Metrics.inc('proxy_to_clsi', 1, { path: action, status })
-      const streamingStarted = Boolean(res.headersSent)
-      if (!streamingStarted) {
-        if (err instanceof RequestFailedError) {
-          res.sendStatus(err.response.status)
-        } else {
-          res.sendStatus(500)
-        }
-      }
-      if (
-        streamingStarted &&
-        reqAborted &&
-        (err.code === 'ERR_STREAM_PREMATURE_CLOSE' ||
-          err.code === 'ERR_STREAM_UNABLE_TO_PIPE')
-      ) {
-        // Ignore noisy spurious error
-        return
-      }
-      if (
-        err instanceof RequestFailedError &&
-        ['sync-to-code', 'sync-to-pdf', 'output-file'].includes(action)
-      ) {
-        // Ignore noisy error
-        // https://github.com/overleaf/internal/issues/15201
-        return
-      }
-      logger.warn(
-        {
-          err,
-          projectId,
-          url,
-          action,
-          reqAborted,
-          streamingStarted,
-          duration,
-        },
-        'CLSI proxy error'
-      )
-    } finally {
-      clearTimeout(timeout)
-    }
-  },
-
   async wordCount(req, res) {
     const { params, query } = parseReq(req, wordCountSchema)
     const projectId = params.Project_id
@@ -747,41 +594,186 @@ const _CompileController = {
   },
 }
 
-async function _getPersistenceOptions(
+async function _downloadFromClsi(
+  projectIdOrSubmissionId,
+  userId,
+  editorId,
+  buildId,
+  file,
+  clsiServerId,
+  action,
   req,
-  projectId,
-  compileGroup,
-  compileBackendClass
+  res
 ) {
-  const { clsiserverid } = req.query
-  const userId = SessionManager.getLoggedInUserId(req)
-  if (clsiserverid && typeof clsiserverid === 'string') {
-    return {
-      qs: { clsiserverid, compileGroup, compileBackendClass },
-      headers: {},
-    }
-  } else {
-    const clsiServerId = await ClsiCookieManager.promises.getServerId(
-      projectId,
-      userId,
-      compileGroup,
-      compileBackendClass
+  const { compileBackendClass } =
+    await CompileManager.promises.getProjectCompileLimits(
+      projectIdOrSubmissionId
     )
-    return {
-      qs: { compileGroup, compileBackendClass },
-      headers: clsiServerId
-        ? {
-            Cookie: new Cookie({
-              key: Settings.clsiCookie.key,
-              value: clsiServerId,
-            }).cookieString(),
-          }
-        : {},
+  const url = getOutputZipURL(
+    projectIdOrSubmissionId,
+    userId,
+    buildId,
+    compileBackendClass,
+    clsiServerId
+  )
+  return await _proxyToClsi(
+    url,
+    projectIdOrSubmissionId,
+    userId,
+    editorId,
+    buildId,
+    file,
+    action,
+    req,
+    res
+  )
+}
+
+async function _downloadFromClsiNginx(
+  projectIdOrSubmissionId,
+  userId,
+  editorId,
+  buildId,
+  file,
+  clsiServerId,
+  action,
+  req,
+  res
+) {
+  const url = getOutputFileURL(
+    projectIdOrSubmissionId,
+    userId,
+    buildId,
+    file,
+    clsiServerId
+  )
+  return await _proxyToClsi(
+    url,
+    projectIdOrSubmissionId,
+    userId,
+    editorId,
+    buildId,
+    file,
+    action,
+    req,
+    res
+  )
+}
+
+async function _proxyToClsi(
+  url,
+  projectIdOrSubmissionId,
+  userId,
+  editorId,
+  buildId,
+  file,
+  action,
+  req,
+  res
+) {
+  const timer = new Metrics.Timer(
+    'proxy_to_clsi',
+    1,
+    { path: action },
+    [0, 100, 1000, 2000, 5000, 10000, 15000, 20000, 30000, 45000, 60000]
+  )
+  Metrics.inc('proxy_to_clsi', 1, { path: action, status: 'start' })
+  const ac = new AbortController()
+  let timeout = setTimeout(() => ac.abort(), 10_000)
+  try {
+    const { stream, response } = await fetchStreamWithResponse(url.href, {
+      method: req.method,
+      signal: ac.signal,
+    })
+    if (req.destroyed) {
+      // The client has disconnected already, avoid trying to write into the broken connection.
+      Metrics.inc('proxy_to_clsi', 1, {
+        path: action,
+        status: 'req-aborted',
+      })
+      stream.destroy(new Error('user aborted the request'))
+      return
+    }
+    Metrics.inc('proxy_to_clsi', 1, {
+      path: action,
+      status: response.status,
+    })
+
+    for (const key of ['Content-Length', 'Content-Type']) {
+      if (response.headers.has(key)) {
+        res.setHeader(key, response.headers.get(key))
+      }
+    }
+
+    // Downloads can take a while on a slow connection, increase timeouts to 10min
+    const TEN_MINUTES_IN_MS = 10 * 60 * 1000
+    res.setTimeout(TEN_MINUTES_IN_MS)
+    clearTimeout(timeout)
+    timeout = setTimeout(() => ac.abort(), TEN_MINUTES_IN_MS)
+
+    // Disable buffering in nginx
+    res.setHeader('X-Accel-Buffering', 'no')
+
+    res.writeHead(response.status)
+    await pipeline(stream, res)
+    timer.labels.status = 'success'
+    timer.done()
+  } catch (err) {
+    if (canTryClsiCacheFallback(req, res, editorId, file, action, err)) {
+      await ClsiCacheController._downloadFromCacheWithParams(
+        req,
+        res,
+        projectIdOrSubmissionId,
+        `${editorId}-${buildId}`,
+        file
+      )
+      return
     }
+    const reqAborted = Boolean(req.destroyed)
+    const status = reqAborted ? 'req-aborted-late' : 'error'
+    timer.labels.status = status
+    const duration = timer.done()
+    Metrics.inc('proxy_to_clsi', 1, { path: action, status })
+    const streamingStarted = Boolean(res.headersSent)
+    if (!streamingStarted) {
+      if (err instanceof RequestFailedError) {
+        res.status(err.response.status).end()
+      } else {
+        res.status(500).end()
+      }
+    }
+    if (
+      streamingStarted &&
+      reqAborted &&
+      (err.code === 'ERR_STREAM_PREMATURE_CLOSE' ||
+        err.code === 'ERR_STREAM_UNABLE_TO_PIPE')
+    ) {
+      // Ignore noisy spurious error
+      return
+    }
+    if (err instanceof RequestFailedError) {
+      // Ignore noisy error: https://github.com/overleaf/internal/issues/15201
+      return
+    }
+    logger.warn(
+      {
+        err,
+        projectId: projectIdOrSubmissionId,
+        userId,
+        url,
+        action,
+        reqAborted,
+        streamingStarted,
+        duration,
+      },
+      'CLSI proxy error'
+    )
+  } finally {
+    clearTimeout(timeout)
   }
 }
 
-function canTryClsiCacheFallback(req, res, action, err) {
+function canTryClsiCacheFallback(req, res, editorId, file, action, err) {
   const reqAborted = Boolean(req.destroyed)
   const streamingStarted = Boolean(res.headersSent)
   return (
@@ -790,15 +782,10 @@ function canTryClsiCacheFallback(req, res, action, err) {
     err.response.status === 404 &&
     !streamingStarted &&
     !reqAborted &&
-    req.params.build_id &&
-    req.query.editorId &&
-    req.params.file &&
-    // clsi-cache only has a small subset of files available outside the tar-ball
+    editorId &&
+    // clsi-cache only has a small subset of files available outside the tar-ball.
     // The ClsiCacheHandler will validate the filename again.
-    (['output.log', 'output.pdf', 'output.synctex.gz'].includes(
-      req.params.file
-    ) ||
-      req.params.file.endsWith('.blg'))
+    ClsiCacheHandler.isAllowedFilename(file)
   )
 }
 
@@ -812,8 +799,8 @@ const CompileController = {
   deleteAuxFiles: expressify(_CompileController.deleteAuxFiles),
   getOutputZipFromClsi: expressify(_CompileController.getOutputZipFromClsi),
   getFileFromClsi: expressify(_CompileController.getFileFromClsi),
-  getFileFromClsiWithoutUser: expressify(
-    _CompileController.getFileFromClsiWithoutUser
+  getFileForSubmissionFromClsi: expressify(
+    _CompileController.getFileForSubmissionFromClsi
   ),
   proxySyncPdf: expressify(_CompileController.proxySyncPdf),
   proxySyncCode: expressify(_CompileController.proxySyncCode),
@@ -822,8 +809,6 @@ const CompileController = {
   _getSafeProjectName: _CompileController._getSafeProjectName,
   _getSplitTestOptions,
   _getUserIdForCompile: _CompileController._getUserIdForCompile,
-  _proxyToClsi: _CompileController._proxyToClsi,
-  _proxyToClsiWithLimits: _CompileController._proxyToClsiWithLimits,
 }
 
 export default CompileController

+ 13 - 19
services/web/app/src/Features/LinkedFiles/ProjectOutputFileAgent.mjs

@@ -158,24 +158,19 @@ function _getFileStream(linkedFileData, userId, callback) {
       return callback(err)
     }
     const sourceProjectId = project._id
-    CompileManager.getProjectCompileLimits(sourceProjectId, (err, limits) => {
-      if (err) return callback(err)
-
-      ClsiManager.getOutputFileStream(
-        sourceProjectId,
-        userId,
-        limits,
-        clsiServerId,
-        buildId,
-        sourceOutputFilePath,
-        (err, readStream) => {
-          if (err) {
-            return callback(err)
-          }
-          callback(null, readStream)
+    ClsiManager.getOutputFileStream(
+      sourceProjectId,
+      userId,
+      clsiServerId,
+      buildId,
+      sourceOutputFilePath,
+      (err, readStream) => {
+        if (err) {
+          return callback(err)
         }
-      )
-    })
+        callback(null, readStream)
+      }
+    )
   })
 }
 
@@ -191,7 +186,7 @@ function _compileAndGetFileStream(linkedFileData, userId, callback) {
       sourceProjectId,
       userId,
       {},
-      (err, status, outputFiles, clsiServerId, limits) => {
+      (err, status, outputFiles, clsiServerId) => {
         if (err) {
           return callback(err)
         }
@@ -209,7 +204,6 @@ function _compileAndGetFileStream(linkedFileData, userId, callback) {
         ClsiManager.getOutputFileStream(
           sourceProjectId,
           userId,
-          limits,
           clsiServerId,
           buildId,
           sourceOutputFilePath,

+ 3 - 22
services/web/app/src/router.mjs

@@ -612,7 +612,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
 
   webRouter.get(
-    '/download/project/:Project_id/build/:buildId/output/cached/:filename',
+    '/download/project/:Project_id/build/:editorBuildId/output/cached/:filename(.*)',
     AuthorizationMiddleware.ensureUserCanReadProject,
     ClsiCacheController.downloadFromCache
   )
@@ -646,16 +646,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
 
   // direct url access to output files for a specific build
   webRouter.get(
-    /^\/project\/([^/]*)\/build\/([0-9a-f-]+)\/output\/(.*)$/,
-    function (req, res, next) {
-      const params = {
-        Project_id: req.params[0],
-        build_id: req.params[1],
-        file: req.params[2],
-      }
-      req.params = params
-      next()
-    },
+    '/project/:Project_id/build/:build_id/output/:file(.*)',
     rateLimiterMiddlewareOutputFiles,
     AuthorizationMiddleware.ensureUserCanReadProject,
     CompileController.getFileFromClsi
@@ -663,17 +654,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
 
   // direct url access to output files for a specific user and build
   webRouter.get(
-    /^\/project\/([^/]*)\/user\/([0-9a-f]+)\/build\/([0-9a-f-]+)\/output\/(.*)$/,
-    function (req, res, next) {
-      const params = {
-        Project_id: req.params[0],
-        user_id: req.params[1],
-        build_id: req.params[2],
-        file: req.params[3],
-      }
-      req.params = params
-      next()
-    },
+    '/project/:Project_id/user/:user_id/build/:build_id/output/:file(.*)',
     rateLimiterMiddlewareOutputFiles,
     AuthorizationMiddleware.ensureUserCanReadProject,
     CompileController.getFileFromClsi

+ 4 - 4
services/web/cypress/support/shared/commands/compile.ts

@@ -1,7 +1,7 @@
 import { v4 as uuid } from 'uuid'
 
 const outputFiles = () => {
-  const build = uuid()
+  const build = uuid().slice(0, 13) // first two groups of UUID
 
   return [
     {
@@ -200,19 +200,19 @@ export const interceptDeferredCompile = (beforeResponse?: () => void) => {
             outputFiles: [
               {
                 path: 'output.pdf',
-                build: '123',
+                build: '1234-5678',
                 url: '/build/123/output.pdf',
                 type: 'pdf',
               },
               {
                 path: 'output.log',
-                build: '123',
+                build: '1234-5678',
                 url: '/build/123/output.log',
                 type: 'log',
               },
               {
                 path: 'output.blg',
-                build: '123',
+                build: '1234-5678',
                 url: '/build/123/output.blg',
                 type: 'log',
               },

+ 6 - 6
services/web/frontend/stories/fixtures/compile.js

@@ -10,37 +10,37 @@ export const dispatchDocChanged = () => {
 export const outputFiles = [
   {
     path: 'output.pdf',
-    build: '123',
+    build: '1234-5678',
     url: '/build/output.pdf',
     type: 'pdf',
   },
   {
     path: 'output.bbl',
-    build: '123',
+    build: '1234-5678',
     url: '/build/output.bbl',
     type: 'bbl',
   },
   {
     path: 'output.bib',
-    build: '123',
+    build: '1234-5678',
     url: '/build/output.bib',
     type: 'bib',
   },
   {
     path: 'example.txt',
-    build: '123',
+    build: '1234-5678',
     url: '/build/example.txt',
     type: 'txt',
   },
   {
     path: 'output.log',
-    build: '123',
+    build: '1234-5678',
     url: '/build/output.log',
     type: 'log',
   },
   {
     path: 'output.blg',
-    build: '123',
+    build: '1234-5678',
     url: '/build/output.blg',
     type: 'blg',
   },

+ 4 - 4
services/web/test/acceptance/src/mocks/MockClsiApi.mjs

@@ -8,16 +8,16 @@ class MockClsiApi extends AbstractMockApi {
         error: null,
         outputFiles: [
           {
-            url: `http://clsi:8080/project/${req.params.project_id}/build/1234/output/output.pdf`,
+            url: `http://clsi:8080/project/${req.params.project_id}/build/1234-5678/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
-            build: 1234,
+            build: '1234-5678',
           },
           {
-            url: `http://clsi:8080/project/${req.params.project_id}/build/1234/output/output.log`,
+            url: `http://clsi:8080/project/${req.params.project_id}/build/1234-5678/output/output.log`,
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: '1234-5678',
           },
         ],
       },

+ 2 - 0
services/web/test/acceptance/src/mocks/MockClsiNginxApi.mjs

@@ -11,6 +11,8 @@ class MockClsiNginxApi extends AbstractMockApi {
           plainTextResponse(res, 'mock-pdf')
         } else if (filename === 'output.log') {
           plainTextResponse(res, 'mock-log')
+        } else if (filename.endsWith('nested.txt')) {
+          plainTextResponse(res, `nested.txt: ${req.originalUrl}`)
         } else {
           res.sendStatus(404)
         }

+ 3 - 3
services/web/test/frontend/features/file-tree/components/file-tree-create/file-tree-modal-create-file.spec.tsx

@@ -230,11 +230,11 @@ describe('<FileTreeModalCreateFile/>', function () {
         status: 'success',
         outputFiles: [
           {
-            build: 'test',
+            build: '1234-5678',
             path: 'baz.jpg',
           },
           {
-            build: 'test',
+            build: '1234-5678',
             path: 'ball.jpg',
           },
         ],
@@ -299,7 +299,7 @@ describe('<FileTreeModalCreateFile/>', function () {
         data: {
           source_project_id: 'project-2',
           source_output_file_path: 'ball.jpg',
-          build_id: 'test',
+          build_id: '1234-5678',
         },
       })
   })

+ 12 - 12
services/web/test/unit/src/Compile/ClsiManager.test.mjs

@@ -289,13 +289,13 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.outputFiles = [
           {
-            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/1234/output/output.pdf`,
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
             build: buildId,
           },
           {
-            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/1234/output/output.log`,
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.log`,
             path: 'output.log',
             type: 'log',
             build: buildId,
@@ -419,13 +419,13 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.outputFiles = [
           {
-            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/1234/output/output.pdf`,
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
             build: buildId,
           },
           {
-            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/1234/output/output.log`,
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.log`,
             path: 'output.log',
             type: 'log',
             build: buildId,
@@ -570,20 +570,20 @@ describe('ClsiManager', function () {
         ctx.contentId = '123-321'
         ctx.outputFiles = [
           {
-            url: `/project/${ctx.project._id}/user/${ctx.user_id}/build/1234/output/output.pdf`,
+            url: `/project/${ctx.project._id}/user/${ctx.user_id}/build/1234-5678/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
-            build: 1234,
+            build: '1234-5678',
             contentId: ctx.contentId,
             ranges: ctx.ranges,
             startXRefTable: ctx.startXRefTable,
             size: ctx.size,
           },
           {
-            url: `/project/${ctx.project._id}/user/${ctx.user_id}/build/1234/output/output.log`,
+            url: `/project/${ctx.project._id}/user/${ctx.user_id}/build/1234-5678/output/output.log`,
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: '1234-5678',
           },
         ]
         ctx.stats = { fooStat: 1 }
@@ -1097,16 +1097,16 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.outputFiles = [
           {
-            url: `/project/${ctx.submissionId}/build/1234/output/output.pdf`,
+            url: `/project/${ctx.submissionId}/build/1234-5678/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
-            build: 1234,
+            build: '1234-5678',
           },
           {
-            url: `/project/${ctx.submissionId}/build/1234/output/output.log`,
+            url: `/project/${ctx.submissionId}/build/1234-5678/output/output.log`,
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: '1234-5678',
           },
         ]
         ctx.responseBody.compile.outputFiles = ctx.outputFiles.map(

+ 243 - 307
services/web/test/unit/src/Compile/CompileController.test.mjs

@@ -5,12 +5,13 @@ import MockResponse from '../helpers/MockResponse.mjs'
 import { Headers } from 'node-fetch'
 import { ReadableString } from '@overleaf/stream-utils'
 import { RequestFailedError } from '@overleaf/fetch-utils'
+import { asZodError } from '@overleaf/validation-tools/testUtils.js'
 
 const modulePath = '../../../../app/src/Features/Compile/CompileController.mjs'
 
 describe('CompileController', function () {
   beforeEach(async function (ctx) {
-    ctx.user_id = 'wat'
+    ctx.user_id = 'aaaaaaaaaaaaaaaaaaaaaaaa'
     ctx.user = {
       _id: ctx.user_id,
       email: 'user@example.com',
@@ -25,7 +26,10 @@ describe('CompileController', function () {
     ctx.CompileManager = {
       promises: {
         compile: sinon.stub(),
-        getProjectCompileLimits: sinon.stub(),
+        getProjectCompileLimits: sinon.stub().resolves({
+          compileBackendClass: 'c3d',
+          compileGroup: 'standard',
+        }),
         syncTeX: sinon.stub(),
       },
     }
@@ -92,6 +96,12 @@ describe('CompileController', function () {
       pipeline: ctx.pipeline,
     }))
 
+    ctx.Metrics = {
+      inc: sinon.stub(),
+      Timer: sinon.stub().returns({ done: sinon.stub(), labels: {} }),
+    }
+    vi.doMock('@overleaf/metrics', () => ({ default: ctx.Metrics }))
+
     vi.doMock('@overleaf/settings', () => ({
       default: ctx.settings,
     }))
@@ -167,7 +177,10 @@ describe('CompileController', function () {
     ctx.CompileController = (await import(modulePath)).default
     ctx.projectId = 'abc123def456abc123def456'
     ctx.build_id = '18fbe9e7564-30dcb2f71250c690'
-    ctx.next = sinon.stub()
+    ctx.next = sinon.stub().callsFake(err => {
+      // Flag unexpected next calls.
+      throw err
+    })
     ctx.req = new MockRequest(vi)
     ctx.res = new MockResponse(vi)
     ctx.res = new MockResponse(vi)
@@ -220,7 +233,7 @@ describe('CompileController', function () {
               ],
               outputFilesArchive: {
                 path: 'output.zip',
-                url: `/project/${ctx.projectId}/user/wat/build/${ctx.build_id}/output/output.zip`,
+                url: `/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.zip`,
                 type: 'zip',
               },
               pdfDownloadDomain: 'https://compiles.overleaf.test',
@@ -265,7 +278,7 @@ describe('CompileController', function () {
               ],
               outputFilesArchive: {
                 path: 'output.zip',
-                url: `/project/${ctx.projectId}/user/wat/build/${ctx.build_id}/output/output.zip`,
+                url: `/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.zip`,
                 type: 'zip',
               },
               outputUrlPrefix: '/zone/b',
@@ -317,7 +330,7 @@ describe('CompileController', function () {
             outputFiles: ctx.outputFiles,
             outputFilesArchive: {
               path: 'output.zip',
-              url: `/project/${ctx.projectId}/user/wat/build/${ctx.build_id}/output/output.zip`,
+              url: `/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.zip`,
               type: 'zip',
             },
           })
@@ -511,13 +524,18 @@ describe('CompileController', function () {
 
   describe('downloadPdf', function () {
     beforeEach(function (ctx) {
-      ctx.CompileController._proxyToClsi = sinon.stub().resolves()
-      ctx.req.params = { Project_id: ctx.projectId }
+      ctx.clsiServerId = 'clsi-server-1'
+      ctx.req.params = {
+        Project_id: ctx.projectId,
+        build_id: ctx.build_id,
+      }
+      ctx.req.query = { clsiserverid: ctx.clsiServerId }
+      ctx.req.session = {}
       ctx.project = { name: 'test namè; 1' }
       ctx.ProjectGetter.promises.getProject = sinon.stub().resolves(ctx.project)
     })
 
-    describe('when downloading for embedding', function () {
+    describe('logged-in', function () {
       beforeEach(async function (ctx) {
         await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next)
       })
@@ -543,36 +561,22 @@ describe('CompileController', function () {
       })
 
       it('should proxy the PDF from the CLSI', function (ctx) {
-        ctx.CompileController._proxyToClsi
-          .calledWith(
-            ctx.projectId,
-            'output-file',
-            `/project/${ctx.projectId}/user/${ctx.user_id}/output/output.pdf`,
-            {},
-            ctx.req,
-            ctx.res
-          )
-          .should.equal(true)
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.pdf?clsiserverid=${ctx.clsiServerId}`
+        )
       })
     })
 
-    describe('when a build-id is provided', function () {
+    describe('anon', function () {
       beforeEach(async function (ctx) {
-        ctx.req.params.build_id = ctx.build_id
+        ctx.SessionManager.getLoggedInUserId.returns(null)
         await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next)
       })
 
-      it('should proxy the PDF from the CLSI, with a build-id', function (ctx) {
-        ctx.CompileController._proxyToClsi
-          .calledWith(
-            ctx.projectId,
-            'output-file',
-            `/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.pdf`,
-            {},
-            ctx.req,
-            ctx.res
-          )
-          .should.equal(true)
+      it('should proxy the PDF from the CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/build/${ctx.build_id}/output/output.pdf?clsiserverid=${ctx.clsiServerId}`
+        )
       })
     })
 
@@ -587,9 +591,8 @@ describe('CompileController', function () {
       })
       it('should return 500', async function (ctx) {
         await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next)
-        // should it be 429 instead?
-        expect(ctx.res.sendStatus).toBeCalledWith(500)
-        ctx.CompileController._proxyToClsi.should.not.have.been.called
+        expect(ctx.res.status).toBeCalledWith(429)
+        ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
       })
     })
 
@@ -599,70 +602,111 @@ describe('CompileController', function () {
       })
       it('should return 500', async function (ctx) {
         await ctx.CompileController.downloadPdf(ctx.req, ctx.res, ctx.next)
-        expect(ctx.res.sendStatus).toBeCalledWith(500)
-        ctx.CompileController._proxyToClsi.should.not.have.been.called
+        expect(ctx.res.status).toBeCalledWith(500)
+        ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
       })
     })
   })
 
-  describe('getFileFromClsiWithoutUser', function () {
+  describe('getOutputZipFromClsi', function () {
     beforeEach(function (ctx) {
-      ctx.submission_id = 'sub-1234'
-      ctx.file = 'output.pdf'
+      ctx.clsiServerId = 'clsi-server-1'
       ctx.req.params = {
-        submission_id: ctx.submission_id,
+        Project_id: ctx.projectId,
         build_id: ctx.build_id,
-        file: ctx.file,
       }
-      ctx.req.body = {}
-      ctx.expected_url = `/project/${ctx.submission_id}/build/${ctx.build_id}/output/${ctx.file}`
-      ctx.CompileController._proxyToClsiWithLimits = sinon.stub()
+      ctx.req.query = { clsiserverid: ctx.clsiServerId }
+      ctx.req.session = {}
+      ctx.project = { name: 'test namè; 1' }
+      ctx.ProjectGetter.promises.getProject = sinon.stub().resolves(ctx.project)
     })
 
-    describe('without limits specified', function () {
+    describe('free user', function () {
       beforeEach(async function (ctx) {
-        await ctx.CompileController.getFileFromClsiWithoutUser(
+        await ctx.CompileController.getOutputZipFromClsi(
           ctx.req,
           ctx.res,
           ctx.next
         )
       })
 
-      it('should proxy to CLSI with correct URL and default limits', function (ctx) {
-        ctx.CompileController._proxyToClsiWithLimits.should.have.been.calledWith(
-          ctx.submission_id,
-          'output-file',
-          ctx.expected_url,
-          {},
-          { compileGroup: 'standard', compileBackendClass: 'c3d' }
+      it('should look up the project', function (ctx) {
+        ctx.ProjectGetter.promises.getProject
+          .calledWith(ctx.projectId, { name: 1 })
+          .should.equal(true)
+      })
+
+      it('should set the content-type of the response to application/zip', function (ctx) {
+        expect(ctx.res.contentType).toBeCalledWith('application/zip')
+      })
+
+      it('should set the content-disposition header with a safe version of the project name', function (ctx) {
+        expect(ctx.res.headers['Content-Disposition']).toEqual(
+          'attachment; filename="test_namè__1-output.zip"'
+        )
+      })
+
+      it('should proxy the PDF from the CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          `${ctx.settings.apis.clsi.url}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.zip?compileBackendClass=c3d&clsiserverid=${ctx.clsiServerId}`
         )
       })
     })
 
-    describe('with limits specified', function () {
-      beforeEach(function (ctx) {
-        ctx.req.body = { compileTimeout: 600, compileGroup: 'special' }
-        ctx.CompileController.getFileFromClsiWithoutUser(
+    describe('premium user', function () {
+      beforeEach(async function (ctx) {
+        ctx.CompileManager.promises.getProjectCompileLimits = sinon
+          .stub()
+          .resolves({
+            compileGroup: 'priority',
+            compileBackendClass: 'c4d',
+          })
+        await ctx.CompileController.getOutputZipFromClsi(
           ctx.req,
           ctx.res,
           ctx.next
         )
       })
 
-      it('should proxy to CLSI with correct URL and specified limits', function (ctx) {
-        ctx.CompileController._proxyToClsiWithLimits.should.have.been.calledWith(
-          ctx.submission_id,
-          'output-file',
-          ctx.expected_url,
-          {},
-          {
-            compileGroup: 'special',
-            compileBackendClass: 'c3d',
-          }
+      it('should proxy the PDF from the CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          `${ctx.settings.apis.clsi.url}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.zip?compileBackendClass=c4d&clsiserverid=${ctx.clsiServerId}`
         )
       })
     })
   })
+
+  describe('getFileForSubmissionFromClsi', function () {
+    beforeEach(function (ctx) {
+      ctx.submission_id = 'sub-1234'
+      ctx.clsiServerId = 'clsi-server-1'
+      ctx.file = 'output.pdf'
+      ctx.req.params = {
+        submissionId: ctx.submission_id,
+        build_id: ctx.build_id,
+        file: ctx.file,
+      }
+      ctx.req.query = { clsiserverid: ctx.clsiServerId }
+    })
+
+    describe('proxy to CLSI with correct URL', function () {
+      beforeEach(async function (ctx) {
+        await ctx.CompileController.getFileForSubmissionFromClsi(
+          ctx.req,
+          ctx.res,
+          ctx.next
+        )
+      })
+
+      it('should proxy to CLSI with correct URL', function (ctx) {
+        const expectedUrl = `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.submission_id}/build/${ctx.build_id}/output/${ctx.file}?clsiserverid=${ctx.clsiServerId}`
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          expectedUrl
+        )
+      })
+    })
+  })
+
   describe('proxySyncCode', function () {
     let file, line, column, imageName, editorId, buildId, clsiServerId
 
@@ -763,248 +807,148 @@ describe('CompileController', function () {
     })
   })
 
-  describe('_proxyToClsi', function () {
+  describe('getFileFromClsi', function () {
     beforeEach(function (ctx) {
-      ctx.req.method = 'mock-method'
-      ctx.req.headers = {
-        Mock: 'Headers',
-        Range: '123-456',
-        'If-Range': 'abcdef',
-        'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT',
+      ctx.clsiServerId = 'clsi-server-1'
+      ctx.req.params = {
+        Project_id: ctx.projectId,
+        build_id: ctx.build_id,
+        file: 'output.blg',
       }
+      ctx.req.query = { clsiserverid: ctx.clsiServerId }
+      ctx.req.session = {}
+      ctx.req.method = 'GET'
     })
 
-    describe('old pdf viewer', function () {
-      describe('user with standard priority', function () {
-        beforeEach(async function (ctx) {
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'standard',
-              compileBackendClass: 'c3d',
-            })
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            (ctx.url = '/test'),
-            { query: 'foo' },
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
-
-        it('should open a request to the CLSI', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=standard&compileBackendClass=c3d&query=foo`
-          )
-        })
-
-        it('should pass the request on to the client', function (ctx) {
-          ctx.pipeline.should.have.been.calledWith(ctx.clsiStream, ctx.res)
-        })
+    describe('when the output.blg exists', function () {
+      beforeEach(async function (ctx) {
+        await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next)
       })
 
-      describe('user with priority compile', function () {
-        beforeEach(async function (ctx) {
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'priority',
-              compileBackendClass: 'c4d',
-            })
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            (ctx.url = '/test'),
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+      it('should open a request to the CLSI download host with compile limits', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.blg?clsiserverid=${ctx.clsiServerId}`
+        )
+      })
 
-        it('should open a request to the CLSI', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`
-          )
-        })
+      it('should pass the response stream on to the client', function (ctx) {
+        ctx.pipeline.should.have.been.calledWith(ctx.clsiStream, ctx.res)
       })
+    })
 
-      describe('when the output.pdf does not exist', function () {
-        beforeEach(async function (ctx) {
-          ctx.req.params.file = 'output.pdf'
-          ctx.req.params.build_id = ctx.build_id
-          ctx.url = `/project/${ctx.projectId}/build/${ctx.build_id}/output/${ctx.req.params.file}`
-          ctx.editorId = '00000000-0000-0000-0000-000000000042'
-          ctx.req.query = { editorId: ctx.editorId }
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'priority',
-              compileBackendClass: 'c4d',
-            })
-          ctx.fetchUtils.fetchStreamWithResponse.rejects(
-            new RequestFailedError(
-              `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`,
-              { method: 'GET' },
-              { status: 404 }
-            )
-          )
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            ctx.url,
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+    describe('when the output.blg traverses up', function () {
+      beforeEach(async function (ctx) {
+        ctx.req.params.file = '../output.blg'
+        ctx.next = sinon.stub()
+        await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next)
+      })
 
-        it('should open a request to the CLSI', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`
-          )
+      it('should reject the request', function (ctx) {
+        ctx.next.should.have.been.calledWithMatch({
+          name: 'ParamsError',
+          cause: asZodError({
+            code: 'custom',
+            path: ['params', 'file'],
+            message: 'path traversal detected',
+          }),
         })
+      })
 
-        it('should fallback to clsi-cache', function (ctx) {
-          ctx.ClsiCacheController._downloadFromCacheWithParams.should.have.been.calledWith(
-            ctx.req,
-            ctx.res,
-            ctx.projectId,
-            `${ctx.editorId}-${ctx.build_id}`,
-            'output.pdf'
-          )
-        })
+      it('should not open a request to CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
       })
-      describe('when the output.stderr does not exist', function () {
-        beforeEach(async function (ctx) {
-          ctx.req.params.file = 'output.stderr'
-          ctx.req.params.build_id = ctx.build_id
-          ctx.url = `/project/${ctx.projectId}/build/${ctx.build_id}/output/${ctx.req.params.file}`
-          ctx.editorId = '00000000-0000-0000-0000-000000000042'
-          ctx.req.query = { editorId: ctx.editorId }
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'priority',
-              compileBackendClass: 'c4d',
-            })
-          ctx.fetchUtils.fetchStreamWithResponse.rejects(
-            new RequestFailedError(
-              `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`,
-              { method: 'GET' },
-              { status: 404 }
-            )
-          )
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            ctx.url,
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+    })
 
-        it('should open a request to the CLSI', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`
-          )
-        })
+    describe('when the buildId traverses up', function () {
+      beforeEach(async function (ctx) {
+        ctx.req.params.build_id = '../..'
+        ctx.next = sinon.stub()
+        await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next)
+      })
 
-        it('should not fallback to clsi-cache', function (ctx) {
-          ctx.ClsiCacheController._downloadFromCacheWithParams.should.not.have
-            .been.called
-          ctx.res.statusCode.should.equal(404)
+      it('should reject the request', function (ctx) {
+        ctx.next.should.have.been.calledWithMatch({
+          name: 'ParamsError',
+          cause: asZodError({
+            origin: 'string',
+            code: 'invalid_format',
+            format: 'regex',
+            pattern: '/^[0-9a-f]+-[0-9a-f]+$/',
+            path: ['params', 'build_id'],
+            message: 'invalid buildId',
+          }),
         })
       })
 
-      describe('user with standard priority via query string', function () {
-        beforeEach(async function (ctx) {
-          ctx.req.query = { compileGroup: 'standard' }
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'standard',
-              compileBackendClass: 'c3d',
-            })
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            (ctx.url = '/test'),
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+      it('should not open a request to CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
+      })
+    })
 
-        it('should open a request to the CLSI', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=standard&compileBackendClass=c3d`
+    describe('when the output.blg does not exist', function () {
+      beforeEach(async function (ctx) {
+        ctx.editorId = '0e546f78-928e-4e8a-b5ea-3136ccf1dc53'
+        ctx.req.query = {
+          clsiserverid: ctx.clsiServerId,
+          editorId: ctx.editorId,
+        }
+        ctx.clsiURL = `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.blg?clsiserverid=${ctx.clsiServerId}`
+        ctx.fetchUtils.fetchStreamWithResponse.rejects(
+          new RequestFailedError(
+            ctx.clsiURL,
+            { method: 'GET' },
+            { status: 404 }
           )
-        })
+        )
+        await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next)
+      })
 
-        it('should pass the request on to the client', function (ctx) {
-          ctx.pipeline.should.have.been.calledWith(ctx.clsiStream, ctx.res)
-        })
+      it('should open a request to the CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          ctx.clsiURL
+        )
       })
 
-      describe('user with non-existent priority via query string', function () {
-        beforeEach(async function (ctx) {
-          ctx.req.query = { compileGroup: 'foobar' }
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'standard',
-              compileBackendClass: 'c3d',
-            })
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            (ctx.url = '/test'),
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+      it('should fallback to clsi-cache', function (ctx) {
+        ctx.ClsiCacheController._downloadFromCacheWithParams.should.have.been.calledWith(
+          ctx.req,
+          ctx.res,
+          ctx.projectId,
+          `${ctx.editorId}-${ctx.build_id}`,
+          'output.blg'
+        )
+      })
+    })
 
-        it('should proxy to the standard url', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=standard&compileBackendClass=c3d`
+    describe('when the output.stderr does not exist', function () {
+      beforeEach(async function (ctx) {
+        ctx.req.params.file = 'output.stderr'
+        ctx.editorId = '0e546f78-928e-4e8a-b5ea-3136ccf1dc53'
+        ctx.req.query = {
+          clsiserverid: ctx.clsiServerId,
+          editorId: ctx.editorId,
+        }
+        ctx.clsiURL = `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/user/${ctx.user_id}/build/${ctx.build_id}/output/output.stderr?clsiserverid=${ctx.clsiServerId}`
+        ctx.fetchUtils.fetchStreamWithResponse.rejects(
+          new RequestFailedError(
+            ctx.clsiURL,
+            { method: 'GET' },
+            { status: 404 }
           )
-        })
+        )
+        await ctx.CompileController.getFileFromClsi(ctx.req, ctx.res, ctx.next)
       })
 
-      describe('user with build parameter via query string', function () {
-        beforeEach(async function (ctx) {
-          ctx.CompileManager.promises.getProjectCompileLimits = sinon
-            .stub()
-            .resolves({
-              compileGroup: 'standard',
-              compileBackendClass: 'c3d',
-            })
-          ctx.req.query = { build: 1234 }
-          await ctx.CompileController._proxyToClsi(
-            ctx.projectId,
-            'output-file',
-            (ctx.url = '/test'),
-            {},
-            ctx.req,
-            ctx.res,
-            ctx.next
-          )
-        })
+      it('should open a request to the CLSI', function (ctx) {
+        ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+          ctx.clsiURL
+        )
+      })
 
-        it('should proxy to the standard url without the build parameter', function (ctx) {
-          ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${ctx.settings.apis.clsi.downloadHost}${ctx.url}?compileGroup=standard&compileBackendClass=c3d`
-          )
-        })
+      it('should not fallback to clsi-cache', function (ctx) {
+        ctx.ClsiCacheController._downloadFromCacheWithParams.should.not.have
+          .been.called
+        expect(ctx.res.statusCode).to.equal(404)
       })
     })
   })
@@ -1030,21 +974,28 @@ describe('CompileController', function () {
   })
 
   describe('compileAndDownloadPdf', function () {
+    const clsiServerId = 'server-1'
+
     beforeEach(function (ctx) {
       ctx.req = {
         params: {
           project_id: ctx.projectId,
         },
+        method: 'GET',
       }
-      ctx.downloadPath = `/project/${ctx.projectId}/build/123/output/output.pdf`
       ctx.CompileManager.promises.compile.resolves({
         status: 'success',
-        outputFiles: [{ path: 'output.pdf', url: ctx.downloadPath }],
+        outputFiles: [{ path: 'output.pdf' }],
+        clsiServerId,
+        buildId: ctx.build_id,
       })
-      ctx.CompileController._proxyToClsi = sinon.stub()
       ctx.res = {
         send: () => {},
         sendStatus: sinon.stub(),
+        writeHead: sinon.stub(),
+        setHeader: sinon.stub(),
+        setTimeout: sinon.stub(),
+        headersSent: false,
       }
     })
 
@@ -1055,28 +1006,11 @@ describe('CompileController', function () {
         .should.equal(true)
     })
 
-    it('should proxy the res to the clsi with correct url', async function (ctx) {
+    it('should proxy the PDF from the CLSI with the correct URL', async function (ctx) {
       await ctx.CompileController.compileAndDownloadPdf(ctx.req, ctx.res)
-      sinon.assert.calledWith(
-        ctx.CompileController._proxyToClsi,
-        ctx.projectId,
-        'output-file',
-        ctx.downloadPath,
-        {},
-        ctx.req,
-        ctx.res
+      ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
+        `${ctx.settings.apis.clsi.downloadHost}/project/${ctx.projectId}/build/${ctx.build_id}/output/output.pdf?clsiserverid=${clsiServerId}`
       )
-
-      ctx.CompileController._proxyToClsi
-        .calledWith(
-          ctx.projectId,
-          'output-file',
-          ctx.downloadPath,
-          {},
-          ctx.req,
-          ctx.res
-        )
-        .should.equal(true)
     })
 
     it('should not download anything on compilation failures', async function (ctx) {
@@ -1087,17 +1021,19 @@ describe('CompileController', function () {
         ctx.next
       )
       ctx.res.sendStatus.should.have.been.calledWith(500)
-      ctx.CompileController._proxyToClsi.should.not.have.been.called
+      ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
     })
 
     it('should not download anything on missing pdf', async function (ctx) {
       ctx.CompileManager.promises.compile.resolves({
         status: 'success',
         outputFiles: [],
+        clsiServerId,
+        buildId: ctx.build_id,
       })
       await ctx.CompileController.compileAndDownloadPdf(ctx.req, ctx.res)
       ctx.res.sendStatus.should.have.been.calledWith(500)
-      ctx.CompileController._proxyToClsi.should.not.have.been.called
+      ctx.fetchUtils.fetchStreamWithResponse.should.not.have.been.called
     })
   })