diff --git a/services/web/app/src/infrastructure/Views.js b/services/web/app/src/infrastructure/Views.js index 8574e1736a..fc60eb8e81 100644 --- a/services/web/app/src/infrastructure/Views.js +++ b/services/web/app/src/infrastructure/Views.js @@ -40,27 +40,104 @@ const PUG_COMPILE_ARGUMENTS = { module: true, } +/** + * @param {string} compiled + * @return {{duplicates: Array, found: Array}} + * @private + */ +function _findAllMetaTags(compiled) { + const inString = /name=(\\?["'`])(ol-.+?)\1/g + const asExpression = /pug\.attr\("name",\s*(["'`])(ol-.+?)\1/g + + const found = new Set() + const duplicates = new Set() + for (const regex of [inString, asExpression]) { + for (const [, , name] of compiled.matchAll(regex)) { + if (found.has(name)) duplicates.add(name) + found.add(name) + } + } + // Special case: Ignore the loop for adding permissions meta tags. + duplicates.delete('ol-cannot-') + return { found: Array.from(found), duplicates: Array.from(duplicates) } +} + +/** + * @param {string} filePath + * @param {string} firstLine + * @return {boolean} + * @private + */ +function _expectMetaFor(filePath, firstLine) { + // no-js pages have no use for ol-meta tags + if (firstLine.match(/extends .*layout\/layout-no-js/)) return false + // plain html pages have no use for ol-meta tags + if (firstLine === 'doctype html') return false + // xml pages do not use meta tags + if (firstLine === 'doctype xml') return false + // view includes should not add meta tags as we cannot trace these easily. + if (Object.values(Settings.viewIncludes).flat().includes(filePath)) { + if ( + filePath === Path.resolve('modules/writefull/app/views/_editor_meta.pug') + ) { + // Special case: The Writefull module adds meta tags to editor, see inline comment there + return true + } + // default case: no meta tags + return false + } + // default to expect meta tags in top-level templates + return true +} + +/** + * @param {string} filePath + * @param {string} compiled + */ +function checkForDuplicateMeta(filePath, compiled) { + const { found, duplicates } = _findAllMetaTags(compiled) + + if (duplicates.length !== 0) { + throw new Error( + `Found duplicate meta tags in ${filePath} (or it's imports): ${Array.from(duplicates)}` + ) + } + const firstLine = fs.readFileSync(filePath, 'utf-8').split('\n', 1)[0] + const expectNoMeta = _expectMetaFor(filePath, firstLine) + if (found.length === 0 && expectNoMeta) { + throw new Error( + `Expected to find meta entries in ${filePath} (or it's imports)` + ) + } + if (!expectNoMeta && found.length !== 0) { + throw new Error( + `Expected to find no meta entries in plain html or 'viewIncludes'. Found ${Array.from(found)} in ${filePath} (or it's imports)` + ) + } +} + function precompileViewsAndCacheToDisk() { const startTime = Date.now() let success = 0 let precompiled = 0 - for (const filename of buildViewList()) { - const precompiledFilename = filename.replace(/\.pug$/, '.js') + for (const filePath of buildViewList()) { + const precompiledFilename = filePath.replace(/\.pug$/, '.js') try { - const src = pug.compileFileClient(filename, PUG_COMPILE_ARGUMENTS) + const compiled = pug.compileFileClient(filePath, PUG_COMPILE_ARGUMENTS) try { - if (fs.readFileSync(precompiledFilename, 'utf-8') === src) { + if (fs.readFileSync(precompiledFilename, 'utf-8') === compiled) { precompiled++ continue } } catch {} - fs.writeFileSync(precompiledFilename, src, { + checkForDuplicateMeta(filePath, compiled) + fs.writeFileSync(precompiledFilename, compiled, { encoding: 'utf-8', mode: 0o644, }) success++ } catch (err) { - logger.err({ err, filename }, 'failed to precompile pug template') + logger.err({ err, filePath }, 'failed to precompile pug template') throw err } } @@ -71,6 +148,11 @@ function precompileViewsAndCacheToDisk() { } module.exports = { + // for tests + PUG_COMPILE_ARGUMENTS, + _expectMetaFor, + _findAllMetaTags, + compileViewIncludes(app) { const viewIncludes = {} for (const [view, paths] of Object.entries(Settings.viewIncludes)) { @@ -92,28 +174,28 @@ module.exports = { let success = 0 let precompiled = 0 let failures = 0 - for (const filename of buildViewList()) { - const precompiledFilename = filename.replace(/\.pug$/, '.js') + for (const filePath of buildViewList()) { + const precompiledFilename = filePath.replace(/\.pug$/, '.js') if (fs.existsSync(precompiledFilename)) { - logger.debug({ filename }, 'loading precompiled pug template') + logger.debug({ filePath }, 'loading precompiled pug template') try { - pug.cache[filename] = require(precompiledFilename) + pug.cache[filePath] = require(precompiledFilename) precompiled++ continue } catch (err) { logger.error( - { filename, err }, + { filePath, err }, 'error loading precompiled pug template' ) failures++ } } try { - logger.warn({ filename }, 'compiling pug template at boot time') - pug.compileFile(filename, PUG_COMPILE_ARGUMENTS) + logger.warn({ filePath }, 'compiling pug template at boot time') + pug.compileFile(filePath, PUG_COMPILE_ARGUMENTS) success++ } catch (err) { - logger.error({ filename, err }, 'error compiling pug template') + logger.error({ filePath, err }, 'error compiling pug template') failures++ } } diff --git a/services/web/test/unit/src/infrastructure/Views.js b/services/web/test/unit/src/infrastructure/Views.js new file mode 100644 index 0000000000..7b601f5d9c --- /dev/null +++ b/services/web/test/unit/src/infrastructure/Views.js @@ -0,0 +1,129 @@ +/* eslint-disable no-template-curly-in-string */ + +const { expect } = require('chai') +const pug = require('pug') +const modulePath = '../../../../app/src/infrastructure/Views.js' +const SandboxedModule = require('sandboxed-module') + +describe('Views', function () { + beforeEach(function () { + this.Views = SandboxedModule.require(modulePath, { + requires: { + '@overleaf/settings': (this.settings = { + viewIncludes: { + someInclude: 'path/to/_include.pug', + }, + }), + }, + }) + }) + + describe('_expectMetaFor', function () { + const cases = [ + { + name: 'no-js', + filename: '500.pug', + firstLine: 'extends ../layout/layout-no-js', + expectMeta: false, + }, + { + name: 'doctype html', + filename: 'user_info_not_found.pug', + firstLine: 'doctype html', + expectMeta: false, + }, + { + name: 'doctype xml', + filename: 'feed.pug', + firstLine: 'doctype xml', + expectMeta: false, + }, + { + name: 'view include', + filename: 'path/to/_include.pug', + firstLine: '//- comment in include', + expectMeta: false, + }, + { + name: 'view include', + filename: 'ide-react.pug', + firstLine: 'extends ../layout-react', + expectMeta: true, + }, + ] + for (const { name, filename, firstLine, expectMeta } of cases) { + it(name, function () { + expect(this.Views._expectMetaFor(filename, firstLine)).to.equal( + expectMeta + ) + }) + } + }) + + describe('_findAllMetaTags', function () { + const cases = [ + { + name: 'simple quote', + src: "meta(name='ol-foo' content=1)", + found: ['ol-foo'], + duplicates: [], + }, + { + name: 'double quote', + src: 'meta(name="ol-foo" content=1)', + found: ['ol-foo'], + duplicates: [], + }, + { + name: 'code quote', + src: 'meta(name=`ol-foo` content=1)', + found: ['ol-foo'], + duplicates: [], + }, + { + name: 'multiple', + src: "meta(name='ol-foo' content=1)\nmeta(name='ol-bar' content=2)", + found: ['ol-foo', 'ol-bar'], + duplicates: [], + }, + { + name: 'computed single', + src: "meta(name='ol-prefix-' + foo content=1)", + found: ['ol-prefix-'], + duplicates: [], + }, + { + name: 'computed double', + src: 'meta(name="ol-prefix-" + foo content=1)', + found: ['ol-prefix-'], + duplicates: [], + }, + { + name: 'computed code', + src: 'meta(name=`ol-prefix-${foo}` content=1)', + found: ['ol-prefix-${foo}'], + duplicates: [], + }, + { + name: 'duplicate', + src: `meta(name='ol-foo' content=1)\nmeta(name="ol-foo")`, + found: ['ol-foo'], + duplicates: ['ol-foo'], + }, + { + name: 'compiled code', + compiled: `pug_html = pug_html + "\u003Cmeta" + (" name=\\"ol-csrfToken\\""+pug.attr("content", csrfToken, true, true)) + "\u003E\u003Cmeta" + (" name=\\"ol-baseAssetPath\\""+pug.attr("content", buildBaseAssetPath(), true, true))`, + found: ['ol-csrfToken', 'ol-baseAssetPath'], + duplicates: [], + }, + ] + for (const { name, compiled, src, found, duplicates } of cases) { + it(name, function () { + const res = this.Views._findAllMetaTags( + compiled || pug.compileClient(src, this.Views.PUG_COMPILE_ARGUMENTS) + ) + expect(res).to.deep.equal({ found, duplicates }) + }) + } + }) +})