From bdbfe70086c78e98f8ca81c9f65f40888cd5adb6 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 15 Dec 2020 12:03:25 +0000 Subject: [PATCH 1/5] rename staticServer to staticCompileServer --- services/clsi/app.js | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index 9402869c04..05bcffce32 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -118,21 +118,25 @@ const ForbidSymlinks = require('./app/js/StaticServerForbidSymlinks') // 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 staticServer = ForbidSymlinks(express.static, Settings.path.compilesDir, { - 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)) +const staticCompileServer = ForbidSymlinks( + express.static, + Settings.path.compilesDir, + { + 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)) + } + return res.set('Content-Type', ContentTypeMapper.map(path)) } - return res.set('Content-Type', ContentTypeMapper.map(path)) } -}) +) app.get( '/project/:project_id/user/:user_id/build/:build_id/output/*', @@ -141,7 +145,7 @@ app.get( req.url = `/${req.params.project_id}-${req.params.user_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticServer(req, res, next) + return staticCompileServer(req, res, next) } ) @@ -154,7 +158,7 @@ app.get('/project/:project_id/build/:build_id/output/*', function ( req.url = `/${req.params.project_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticServer(req, res, next) + return staticCompileServer(req, res, next) }) app.get('/project/:project_id/user/:user_id/output/*', function ( @@ -164,7 +168,7 @@ app.get('/project/:project_id/user/:user_id/output/*', function ( ) { // for specific user get the path to the top level file req.url = `/${req.params.project_id}-${req.params.user_id}/${req.params[0]}` - return staticServer(req, res, next) + return staticCompileServer(req, res, next) }) app.get('/project/:project_id/output/*', function (req, res, next) { @@ -179,7 +183,7 @@ app.get('/project/:project_id/output/*', function (req, res, next) { } else { req.url = `/${req.params.project_id}/${req.params[0]}` } - return staticServer(req, res, next) + return staticCompileServer(req, res, next) }) app.get('/oops', function (req, res, next) { From 692dbc8d6bde57730129b76b4456ca8a47a2dbcb Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 15 Dec 2020 14:59:05 +0000 Subject: [PATCH 2/5] add output directory --- services/clsi/Dockerfile | 4 ++-- services/clsi/app.js | 24 ++++++++++++++++++++-- services/clsi/app/js/CompileManager.js | 6 ++++++ services/clsi/app/js/OutputCacheManager.js | 24 ++++++++++++++-------- services/clsi/config/settings.defaults.js | 1 + 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/services/clsi/Dockerfile b/services/clsi/Dockerfile index 4be4353cb9..7eaa03f814 100644 --- a/services/clsi/Dockerfile +++ b/services/clsi/Dockerfile @@ -22,7 +22,7 @@ COPY . /app FROM base COPY --from=app /app /app -RUN mkdir -p cache compiles db \ -&& chown node:node cache compiles db +RUN mkdir -p cache compiles db output \ + && chown node:node cache compiles db output CMD ["node", "--expose-gc", "app.js"] diff --git a/services/clsi/app.js b/services/clsi/app.js index 05bcffce32..c87edeb3f2 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -138,6 +138,26 @@ const staticCompileServer = ForbidSymlinks( } ) +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)) + } + return res.set('Content-Type', ContentTypeMapper.map(path)) + } + } +) + app.get( '/project/:project_id/user/:user_id/build/:build_id/output/*', function (req, res, next) { @@ -145,7 +165,7 @@ app.get( req.url = `/${req.params.project_id}-${req.params.user_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticCompileServer(req, res, next) + return staticOutputServer(req, res, next) } ) @@ -158,7 +178,7 @@ app.get('/project/:project_id/build/:build_id/output/*', function ( req.url = `/${req.params.project_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticCompileServer(req, res, next) + return staticOutputServer(req, res, next) }) app.get('/project/:project_id/user/:user_id/output/*', function ( diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 68edde49b0..5dab554ff2 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -46,6 +46,9 @@ const getCompileName = function (project_id, user_id) { const getCompileDir = (project_id, user_id) => Path.join(Settings.path.compilesDir, getCompileName(project_id, user_id)) +const getOutputDir = (project_id, user_id) => + Path.join(Settings.path.outputDir, getCompileName(project_id, user_id)) + module.exports = CompileManager = { doCompileWithLock(request, callback) { if (callback == null) { @@ -72,6 +75,8 @@ module.exports = CompileManager = { callback = function (error, outputFiles) {} } const compileDir = getCompileDir(request.project_id, request.user_id) + const outputDir = getOutputDir(request.project_id, request.user_id) + let timer = new Metrics.Timer('write-to-disk') logger.log( { project_id: request.project_id, user_id: request.user_id }, @@ -294,6 +299,7 @@ module.exports = CompileManager = { return OutputCacheManager.saveOutputFiles( outputFiles, compileDir, + outputDir, (error, newOutputFiles) => callback(null, newOutputFiles) ) } diff --git a/services/clsi/app/js/OutputCacheManager.js b/services/clsi/app/js/OutputCacheManager.js index b34dea7994..8aefb9b77a 100644 --- a/services/clsi/app/js/OutputCacheManager.js +++ b/services/clsi/app/js/OutputCacheManager.js @@ -26,8 +26,8 @@ const crypto = require('crypto') const OutputFileOptimiser = require('./OutputFileOptimiser') module.exports = OutputCacheManager = { - CACHE_SUBDIR: '.cache/clsi', - ARCHIVE_SUBDIR: '.archive/clsi', + CACHE_SUBDIR: 'generated-files', + ARCHIVE_SUBDIR: 'archived-logs', // build id is HEXDATE-HEXRANDOM from Date.now()and RandomBytes // for backwards compatibility, make the randombytes part optional BUILD_REGEX: /^[0-9a-f]+(-[0-9a-f]+)?$/, @@ -59,7 +59,7 @@ module.exports = OutputCacheManager = { }) }, - saveOutputFiles(outputFiles, compileDir, callback) { + saveOutputFiles(outputFiles, compileDir, outputDir, callback) { if (callback == null) { callback = function (error) {} } @@ -70,22 +70,29 @@ module.exports = OutputCacheManager = { return OutputCacheManager.saveOutputFilesInBuildDir( outputFiles, compileDir, + outputDir, buildId, callback ) }) }, - saveOutputFilesInBuildDir(outputFiles, compileDir, buildId, callback) { + saveOutputFilesInBuildDir( + outputFiles, + compileDir, + outputDir, + buildId, + callback + ) { // make a compileDir/CACHE_SUBDIR/build_id directory and // copy all the output files into it if (callback == null) { callback = function (error) {} } - const cacheRoot = Path.join(compileDir, OutputCacheManager.CACHE_SUBDIR) + const cacheRoot = Path.join(outputDir, OutputCacheManager.CACHE_SUBDIR) // Put the files into a new cache subdirectory const cacheDir = Path.join( - compileDir, + outputDir, OutputCacheManager.CACHE_SUBDIR, buildId ) @@ -102,6 +109,7 @@ module.exports = OutputCacheManager = { OutputCacheManager.archiveLogs( outputFiles, compileDir, + outputDir, buildId, function (err) { if (err != null) { @@ -198,12 +206,12 @@ module.exports = OutputCacheManager = { }) }, - archiveLogs(outputFiles, compileDir, buildId, callback) { + archiveLogs(outputFiles, compileDir, outputDir, buildId, callback) { if (callback == null) { callback = function (error) {} } const archiveDir = Path.join( - compileDir, + outputDir, OutputCacheManager.ARCHIVE_SUBDIR, buildId ) diff --git a/services/clsi/config/settings.defaults.js b/services/clsi/config/settings.defaults.js index 823f1f737f..72c3471ba9 100644 --- a/services/clsi/config/settings.defaults.js +++ b/services/clsi/config/settings.defaults.js @@ -27,6 +27,7 @@ module.exports = { path: { compilesDir: Path.resolve(__dirname, '../compiles'), + outputDir: Path.resolve(__dirname, '../output'), clsiCacheDir: Path.resolve(__dirname, '../cache'), synctexBaseDir(projectId) { return Path.join(this.compilesDir, projectId) From 565cd53eb58a18f3f0a7ee71f117a18434fb4202 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 16 Dec 2020 11:18:18 +0000 Subject: [PATCH 3/5] add git ignore for output directory --- services/clsi/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/services/clsi/.gitignore b/services/clsi/.gitignore index b32ea20954..dee3eca1f5 100644 --- a/services/clsi/.gitignore +++ b/services/clsi/.gitignore @@ -2,6 +2,7 @@ node_modules test/acceptance/fixtures/tmp compiles +output .DS_Store *~ cache From b5346658b066bfa5c99f108a0601f38764ca00ec Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 16 Dec 2020 15:14:18 +0000 Subject: [PATCH 4/5] clear output directory when clearing project --- services/clsi/app/js/CompileManager.js | 13 +++++++++++-- .../clsi/test/unit/js/CompileManagerTests.js | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 5dab554ff2..25741005f8 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -329,6 +329,7 @@ module.exports = CompileManager = { } const compileDir = getCompileDir(project_id, user_id) + const outputDir = getOutputDir(project_id, user_id) return CompileManager._checkDirectory(compileDir, function (err, exists) { if (err != null) { @@ -338,7 +339,13 @@ module.exports = CompileManager = { return callback() } // skip removal if no directory present - const proc = child_process.spawn('rm', ['-r', compileDir]) + const proc = child_process.spawn('rm', [ + '-r', + '-f', + '--', + compileDir, + outputDir + ]) proc.on('error', callback) @@ -349,7 +356,9 @@ module.exports = CompileManager = { if (code === 0) { return callback(null) } else { - return callback(new Error(`rm -r ${compileDir} failed: ${stderr}`)) + return callback( + new Error(`rm -r ${compileDir} ${outputDir} failed: ${stderr}`) + ) } }) }) diff --git a/services/clsi/test/unit/js/CompileManagerTests.js b/services/clsi/test/unit/js/CompileManagerTests.js index cb85f2f131..917dfe8cb7 100644 --- a/services/clsi/test/unit/js/CompileManagerTests.js +++ b/services/clsi/test/unit/js/CompileManagerTests.js @@ -34,7 +34,8 @@ describe('CompileManager', function () { './OutputCacheManager': (this.OutputCacheManager = {}), 'settings-sharelatex': (this.Settings = { path: { - compilesDir: '/compiles/dir' + compilesDir: '/compiles/dir', + outputDir: '/output/dir' }, synctexBaseDir() { return '/compile' @@ -166,6 +167,7 @@ describe('CompileManager', function () { this.env = {} this.Settings.compileDir = 'compiles' this.compileDir = `${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id}` + this.outputDir = `${this.Settings.path.outputDir}/${this.project_id}-${this.user_id}` this.ResourceWriter.syncResourcesToDisk = sinon .stub() .callsArgWith(2, null, this.resources) @@ -175,7 +177,7 @@ describe('CompileManager', function () { .callsArgWith(2, null, this.output_files) this.OutputCacheManager.saveOutputFiles = sinon .stub() - .callsArgWith(2, null, this.build_files) + .callsArgWith(3, null, this.build_files) this.DraftModeManager.injectDraftMode = sinon.stub().callsArg(1) return (this.TikzManager.checkMainFile = sinon.stub().callsArg(3, false)) }) @@ -312,7 +314,10 @@ describe('CompileManager', function () { return this.child_process.spawn .calledWith('rm', [ '-r', - `${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id}` + '-f', + '--', + `${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id}`, + `${this.Settings.path.outputDir}/${this.project_id}-${this.user_id}` ]) .should.equal(true) }) @@ -348,7 +353,10 @@ describe('CompileManager', function () { return this.child_process.spawn .calledWith('rm', [ '-r', - `${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id}` + '-f', + '--', + `${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id}`, + `${this.Settings.path.outputDir}/${this.project_id}-${this.user_id}` ]) .should.equal(true) }) @@ -357,7 +365,7 @@ describe('CompileManager', function () { this.callback.calledWithExactly(sinon.match(Error)).should.equal(true) this.callback.args[0][0].message.should.equal( - `rm -r ${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id} failed: ${this.error}` + `rm -r ${this.Settings.path.compilesDir}/${this.project_id}-${this.user_id} ${this.Settings.path.outputDir}/${this.project_id}-${this.user_id} failed: ${this.error}` ) }) }) From 6fa081522d6ec810b0778f06789eedbb57abeda0 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 22 Jan 2021 11:03:47 +0000 Subject: [PATCH 5/5] add a warning for requests without build id --- services/clsi/app.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/clsi/app.js b/services/clsi/app.js index c87edeb3f2..930ebc3e1c 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -187,11 +187,13 @@ app.get('/project/:project_id/user/:user_id/output/*', function ( next ) { // for specific user get the path to the top level file + logger.warn({ url: req.url }, 'direct request for file in compile directory') req.url = `/${req.params.project_id}-${req.params.user_id}/${req.params[0]}` return staticCompileServer(req, res, next) }) app.get('/project/:project_id/output/*', function (req, res, next) { + logger.warn({ url: req.url }, 'direct request for file in compile directory') if ( (req.query != null ? req.query.build : undefined) != null && req.query.build.match(OutputCacheManager.BUILD_REGEX)