Przeglądaj źródła

[clsi] remove unused endpoints for downloading output files (#31692)

GitOrigin-RevId: a0cac10f3585414779b026f38c2af2773c80082f
Jakob Ackermann 5 miesięcy temu
rodzic
commit
eca31afb4a

+ 0 - 76
services/clsi/app.js

@@ -2,27 +2,21 @@
 import '@overleaf/metrics/initialize.js'
 
 import CompileController from './app/js/CompileController.js'
-import ContentController from './app/js/ContentController.js'
 import Settings from '@overleaf/settings'
 import logger from '@overleaf/logger'
 import LoggerSerializers from './app/js/LoggerSerializers.js'
 
 import Metrics from '@overleaf/metrics'
 import smokeTest from './test/smoke/js/SmokeTests.js'
-import ContentTypeMapper from './app/js/ContentTypeMapper.js'
 import Errors from './app/js/Errors.js'
 import OutputController from './app/js/OutputController.js'
-import Path from 'node:path'
 
 import ProjectPersistenceManager from './app/js/ProjectPersistenceManager.js'
 import OutputCacheManager from './app/js/OutputCacheManager.js'
-import ContentCacheManager from './app/js/ContentCacheManager.js'
 
 import express from 'express'
 import bodyParser from 'body-parser'
 
-import ForbidSymlinks from './app/js/StaticServerForbidSymlinks.js'
-
 import net from 'node:net'
 import os from 'node:os'
 import OError from '@overleaf/o-error'
@@ -75,22 +69,6 @@ app.param('build_id', function (req, res, next, buildId) {
   }
 })
 
-app.param('contentId', function (req, res, next, contentId) {
-  if (contentId?.match(OutputCacheManager.CONTENT_REGEX)) {
-    next()
-  } else {
-    next(new OError('invalid content id', { contentId }))
-  }
-})
-
-app.param('hash', function (req, res, next, hash) {
-  if (hash?.match(ContentCacheManager.HASH_REGEX)) {
-    next()
-  } else {
-    next(new OError('invalid hash', { hash }))
-  }
-})
-
 app.post(
   '/project/:project_id/compile',
   bodyParser.json({ limit: Settings.compileSizeLimit }),
@@ -130,29 +108,6 @@ app.get(
   CompileController.wordcount
 )
 
-// create a static server which does not allow access to any symlinks
-// avoids possible mismatch of root directory between middleware check
-// and serving the files
-const staticOutputServer = ForbidSymlinks(
-  express.static,
-  Settings.path.outputDir,
-  {
-    setHeaders(res, path, stat) {
-      if (Path.basename(path) === 'output.pdf') {
-        // Calculate an etag in the same way as nginx
-        // https://github.com/tj/send/issues/65
-        const etag = (path, stat) =>
-          `"${Math.ceil(+stat.mtime / 1000).toString(16)}` +
-          '-' +
-          Number(stat.size).toString(16) +
-          '"'
-        res.set('Etag', etag(path, stat))
-      }
-      res.set('Content-Type', ContentTypeMapper.map(path))
-    },
-  }
-)
-
 // This needs to be before GET /project/:project_id/build/:build_id/output/*
 app.get(
   '/project/:project_id/build/:build_id/output/output.zip',
@@ -167,37 +122,6 @@ app.get(
   OutputController.createOutputZip
 )
 
-app.get(
-  '/project/:project_id/user/:user_id/build/:build_id/output/*',
-  function (req, res, next) {
-    // for specific build get the path from the OutputCacheManager (e.g. .clsi/buildId)
-    req.url =
-      `/${req.params.project_id}-${req.params.user_id}/` +
-      OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`)
-    staticOutputServer(req, res, next)
-  }
-)
-
-app.get(
-  '/project/:projectId/content/:contentId/:hash',
-  ContentController.getPdfRange
-)
-app.get(
-  '/project/:projectId/user/:userId/content/:contentId/:hash',
-  ContentController.getPdfRange
-)
-
-app.get(
-  '/project/:project_id/build/:build_id/output/*',
-  function (req, res, next) {
-    // for specific build get the path from the OutputCacheManager (e.g. .clsi/buildId)
-    req.url =
-      `/${req.params.project_id}/` +
-      OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`)
-    staticOutputServer(req, res, next)
-  }
-)
-
 app.get('/status', (req, res, next) => res.send('CLSI is alive\n'))
 
 Settings.processTooOld = false

+ 0 - 38
services/clsi/app/js/ContentTypeMapper.js

@@ -1,38 +0,0 @@
-/* eslint-disable
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-import Path from 'node:path'
-let ContentTypeMapper
-
-// here we coerce html, css and js to text/plain,
-// otherwise choose correct mime type based on file extension,
-// falling back to octet-stream
-export default ContentTypeMapper = {
-  map(path) {
-    switch (Path.extname(path)) {
-      case '.txt':
-      case '.html':
-      case '.js':
-      case '.css':
-      case '.svg':
-        return 'text/plain'
-      case '.csv':
-        return 'text/csv'
-      case '.pdf':
-        return 'application/pdf'
-      case '.png':
-        return 'image/png'
-      case '.jpg':
-      case '.jpeg':
-        return 'image/jpeg'
-      case '.tiff':
-        return 'image/tiff'
-      case '.gif':
-        return 'image/gif'
-      default:
-        return 'application/octet-stream'
-    }
-  },
-}

+ 0 - 89
services/clsi/app/js/StaticServerForbidSymlinks.js

@@ -1,89 +0,0 @@
-/* eslint-disable
-    no-cond-assign,
-    no-unused-vars,
-    n/no-deprecated-api,
-*/
-// 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 Path from 'node:path'
-import fs from 'node:fs'
-import Settings from '@overleaf/settings'
-import logger from '@overleaf/logger'
-let ForbidSymlinks
-
-export default ForbidSymlinks = function (staticFn, root, options) {
-  const expressStatic = staticFn(root, options)
-  const basePath = Path.resolve(root)
-  return function (req, res, next) {
-    let file, projectId, result
-    const path = req.url
-    // check that the path is of the form /project_id_or_name/path/to/file.log
-    if ((result = path.match(/^\/([a-zA-Z0-9_-]+)\/(.*)$/s))) {
-      projectId = result[1]
-      file = result[2]
-      if (path !== `/${projectId}/${file}`) {
-        logger.warn({ path }, 'unrecognized file request')
-        return res.sendStatus(404)
-      }
-    } else {
-      logger.warn({ path }, 'unrecognized file request')
-      return res.sendStatus(404)
-    }
-    // check that the file does not use a relative path
-    for (const dir of Array.from(file.split('/'))) {
-      if (dir === '..') {
-        logger.warn({ path }, 'attempt to use a relative path')
-        return res.sendStatus(404)
-      }
-    }
-    // check that the requested path is normalized
-    const requestedFsPath = `${basePath}/${projectId}/${file}`
-    if (requestedFsPath !== Path.normalize(requestedFsPath)) {
-      logger.error(
-        { path: requestedFsPath },
-        'requestedFsPath is not normalized'
-      )
-      return res.sendStatus(404)
-    }
-    // check that the requested path is not a symlink
-    return fs.realpath(requestedFsPath, function (err, realFsPath) {
-      if (err != null) {
-        if (err.code === 'ENOENT') {
-          return res.sendStatus(404)
-        } else {
-          logger.error(
-            {
-              err,
-              requestedFsPath,
-              realFsPath,
-              path: req.params[0],
-              projectId: req.params.project_id,
-            },
-            'error checking file access'
-          )
-          return res.sendStatus(500)
-        }
-      } else if (requestedFsPath !== realFsPath) {
-        logger.warn(
-          {
-            requestedFsPath,
-            realFsPath,
-            path: req.params[0],
-            projectId: req.params.project_id,
-          },
-          'trying to access a different file (symlink), aborting'
-        )
-        return res.sendStatus(404)
-      } else {
-        return expressStatic(req, res, next)
-      }
-    })
-  }
-}

+ 0 - 69
services/clsi/test/unit/js/ContentTypeMapper.test.js

@@ -1,69 +0,0 @@
-import { describe, beforeEach, it } from 'vitest'
-import path from 'node:path'
-
-const modulePath = path.join(
-  import.meta.dirname,
-  '../../../app/js/ContentTypeMapper'
-)
-
-describe('ContentTypeMapper', function () {
-  beforeEach(async function (ctx) {
-    return (ctx.ContentTypeMapper = (await import(modulePath)).default)
-  })
-
-  return describe('map', function () {
-    it('should map .txt to text/plain', function (ctx) {
-      const contentType = ctx.ContentTypeMapper.map('example.txt')
-      return contentType.should.equal('text/plain')
-    })
-
-    it('should map .csv to text/csv', function (ctx) {
-      const contentType = ctx.ContentTypeMapper.map('example.csv')
-      return contentType.should.equal('text/csv')
-    })
-
-    it('should map .pdf to application/pdf', function (ctx) {
-      const contentType = ctx.ContentTypeMapper.map('example.pdf')
-      return contentType.should.equal('application/pdf')
-    })
-
-    it('should fall back to octet-stream', function (ctx) {
-      const contentType = ctx.ContentTypeMapper.map('example.unknown')
-      return contentType.should.equal('application/octet-stream')
-    })
-
-    describe('coercing web files to plain text', function () {
-      it('should map .js to plain text', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.js')
-        return contentType.should.equal('text/plain')
-      })
-
-      it('should map .html to plain text', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.html')
-        return contentType.should.equal('text/plain')
-      })
-
-      return it('should map .css to plain text', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.css')
-        return contentType.should.equal('text/plain')
-      })
-    })
-
-    return describe('image files', function () {
-      it('should map .png to image/png', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.png')
-        return contentType.should.equal('image/png')
-      })
-
-      it('should map .jpeg to image/jpeg', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.jpeg')
-        return contentType.should.equal('image/jpeg')
-      })
-
-      return it('should map .svg to text/plain to protect against XSS (SVG can execute JS)', function (ctx) {
-        const contentType = ctx.ContentTypeMapper.map('example.svg')
-        return contentType.should.equal('text/plain')
-      })
-    })
-  })
-})

+ 0 - 270
services/clsi/test/unit/js/StaticServerForbidSymlinks.test.js

@@ -1,270 +0,0 @@
-import { vi, describe, beforeEach, it } from 'vitest'
-
-import path from 'node:path'
-import sinon from 'sinon'
-const modulePath = path.join(
-  import.meta.dirname,
-  '../../../app/js/StaticServerForbidSymlinks'
-)
-
-describe('StaticServerForbidSymlinks', function () {
-  beforeEach(async function (ctx) {
-    ctx.settings = {
-      path: {
-        compilesDir: '/compiles/here',
-      },
-    }
-
-    ctx.fs = {}
-
-    vi.doMock('@overleaf/settings', () => ({
-      default: ctx.settings,
-    }))
-
-    vi.doMock('fs', () => ({
-      default: ctx.fs,
-    }))
-
-    ctx.ForbidSymlinks = (await import(modulePath)).default
-
-    ctx.dummyStatic = (rootDir, options) => (req, res, next) =>
-      // console.log "dummyStatic serving file", rootDir, "called with", req.url
-      // serve it
-      next()
-
-    ctx.StaticServerForbidSymlinks = ctx.ForbidSymlinks(
-      ctx.dummyStatic,
-      ctx.settings.path.compilesDir
-    )
-    ctx.req = {
-      params: {
-        project_id: '12345',
-      },
-    }
-
-    ctx.res = {}
-    ctx.req.url = '/12345/output.pdf'
-  })
-
-  describe('sending a normal file through', function () {
-    beforeEach(function (ctx) {
-      ctx.fs.realpath = sinon
-        .stub()
-        .callsArgWith(
-          1,
-          null,
-          `${ctx.settings.path.compilesDir}/${ctx.req.params.project_id}/output.pdf`
-        )
-    })
-
-    it('should call next', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(200)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res, err => {
-          if (err) reject(err)
-          resolve()
-        })
-      })
-    })
-  })
-
-  describe('with a missing file', function () {
-    beforeEach(function (ctx) {
-      ctx.fs.realpath = sinon
-        .stub()
-        .callsArgWith(
-          1,
-          { code: 'ENOENT' },
-          `${ctx.settings.path.compilesDir}/${ctx.req.params.project_id}/unknown.pdf`
-        )
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a new line', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/12345/output.pdf\nother file'
-      ctx.fs.realpath = sinon.stub().yields()
-    })
-
-    it('should process the correct file', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = () => {
-          ctx.fs.realpath.should.have.been.calledWith(
-            `${ctx.settings.path.compilesDir}/12345/output.pdf\nother file`
-          )
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a symlink file', function () {
-    beforeEach(function (ctx) {
-      ctx.fs.realpath = sinon
-        .stub()
-        .callsArgWith(1, null, `/etc/${ctx.req.params.project_id}/output.pdf`)
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a relative file', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/12345/../67890/output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a unnormalized file containing .', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/12345/foo/./output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a file containing an empty path', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/12345/foo//output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a non-project file', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/.foo/output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a file outside the compiledir', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/../bar/output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a file with no leading /', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = './../bar/output.pdf'
-    })
-
-    it('should send a 404', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(404)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-
-  describe('with a github style path', function () {
-    beforeEach(function (ctx) {
-      ctx.req.url = '/henryoswald-latex_example/output/output.log'
-      ctx.fs.realpath = sinon
-        .stub()
-        .callsArgWith(
-          1,
-          null,
-          `${ctx.settings.path.compilesDir}/henryoswald-latex_example/output/output.log`
-        )
-    })
-
-    it('should call next', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(200)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res, err => {
-          if (err) reject(err)
-          resolve()
-        })
-      })
-    })
-  })
-
-  describe('with an error from fs.realpath', function () {
-    beforeEach(function (ctx) {
-      ctx.fs.realpath = sinon.stub().callsArgWith(1, 'error')
-    })
-
-    it('should send a 500', async function (ctx) {
-      await new Promise((resolve, reject) => {
-        ctx.res.sendStatus = function (resCode) {
-          resCode.should.equal(500)
-          resolve()
-        }
-        ctx.StaticServerForbidSymlinks(ctx.req, ctx.res)
-      })
-    })
-  })
-})