Merge pull request #21235 from overleaf/jpa-pug-lint

[web] detect duplicate meta tags in pug templates

GitOrigin-RevId: 0f1462b125244bf6da0a93438f44f9dd5a8d0cce

services/web/app/src/infrastructure/Views.js

@@ -40,27 +40,104 @@ const PUG_COMPILE_ARGUMENTS = {
   module: true,
 }
 
+/**
+ * @param {string} compiled
+ * @return {{duplicates: Array<string>, found: Array<string>}}
+ * @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++
       }
     }

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 })
+      })
+    }
+  })
+})