Escape Nunjucks for special tags (#1049)

* Allow nunjucks that is called multiple times to support escaping for special tags * Remove the need to prune raw tags, simplify logic of replacement * Fix style and remove nested nunjucks escape test
MarkBind · Mar 21, 2020 · 7ba15e9 · 7ba15e9
1 parent 2ee765b
commit 7ba15e9
Show file tree

Hide file tree

Showing 9 changed files with 124 additions and 33 deletions.
diff --git a/src/Page.js b/src/Page.js
@@ -6,6 +6,7 @@ const nunjucks = require('nunjucks');
 const path = require('path');
 const pathIsInside = require('path-is-inside');
 const Promise = require('bluebird');
+const nunjuckUtils = require('./lib/markbind/src/utils/nunjuckUtils');
 
 const _ = {};
 _.isString = require('lodash/isString');
@@ -440,7 +441,7 @@ class Page {
     // Retrieve Expressive Layouts page and insert content
       fs.readFileAsync(layoutPagePath, 'utf8')
         .then(result => markbinder.includeData(layoutPagePath, result, layoutFileConfig))
-        .then(result => nj.renderString(result, template))
+        .then(result => nunjuckUtils.renderEscaped(nj, result, template))
         .then((result) => {
           this.collectIncludedFiles(markbinder.getDynamicIncludeSrc());
           this.collectIncludedFiles(markbinder.getStaticIncludeSrc());
@@ -481,7 +482,7 @@ class Page {
     // Map variables
     const newBaseUrl = Page.calculateNewBaseUrl(this.sourcePath, this.rootPath, this.baseUrlMap) || '';
     const userDefinedVariables = this.userDefinedVariablesMap[path.join(this.rootPath, newBaseUrl)];
-    return `${nunjucks.renderString(headerContent, userDefinedVariables)}\n${pageData}`;
+    return `${nunjuckUtils.renderEscaped(nunjucks, headerContent, userDefinedVariables)}\n${pageData}`;
   }
 
   /**
@@ -511,7 +512,7 @@ class Page {
     // Map variables
     const newBaseUrl = Page.calculateNewBaseUrl(this.sourcePath, this.rootPath, this.baseUrlMap) || '';
     const userDefinedVariables = this.userDefinedVariablesMap[path.join(this.rootPath, newBaseUrl)];
-    return `${pageData}\n${nunjucks.renderString(footerContent, userDefinedVariables)}`;
+    return `${pageData}\n${nunjuckUtils.renderEscaped(nunjucks, footerContent, userDefinedVariables)}`;
   }
 
   /**
@@ -548,7 +549,7 @@ class Page {
     // Map variables
     const newBaseUrl = Page.calculateNewBaseUrl(this.sourcePath, this.rootPath, this.baseUrlMap) || '';
     const userDefinedVariables = this.userDefinedVariablesMap[path.join(this.rootPath, newBaseUrl)];
-    const siteNavMappedData = nunjucks.renderString(siteNavContent, userDefinedVariables);
+    const siteNavMappedData = nunjuckUtils.renderEscaped(nunjucks, siteNavContent, userDefinedVariables);
     // Convert to HTML
     const siteNavDataSelector = cheerio.load(siteNavMappedData);
     if (siteNavDataSelector('navigation').length > 1) {
@@ -698,12 +699,12 @@ class Page {
       // Map variables
       const newBaseUrl = Page.calculateNewBaseUrl(this.sourcePath, this.rootPath, this.baseUrlMap) || '';
       const userDefinedVariables = this.userDefinedVariablesMap[path.join(this.rootPath, newBaseUrl)];
-      const headFileMappedData = nunjucks.renderString(headFileContent, userDefinedVariables)
+      const headFileMappedData = nunjuckUtils.renderEscaped(nunjucks, headFileContent, userDefinedVariables)
         .trim();
       // Split top and bottom contents
       const $ = cheerio.load(headFileMappedData, { xmlMode: false });
       if ($('head-top').length) {
-        collectedTopContent.push(nunjucks.renderString($('head-top')
+        collectedTopContent.push(nunjuckUtils.renderEscaped(nunjucks, $('head-top')
           .html(), {
           baseUrl,
           hostBaseUrl,
@@ -714,7 +715,7 @@ class Page {
         $('head-top')
           .remove();
       }
-      collectedBottomContent.push(nunjucks.renderString($.html(), {
+      collectedBottomContent.push(nunjuckUtils.renderEscaped(nunjucks, $.html(), {
         baseUrl,
         hostBaseUrl,
       })

diff --git a/src/Site.js b/src/Site.js
@@ -8,6 +8,7 @@ const Promise = require('bluebird');
 const ProgressBar = require('progress');
 const walkSync = require('walk-sync');
 const MarkBind = require('./lib/markbind/src/parser');
+const nunjuckUtils = require('./lib/markbind/src/utils/nunjuckUtils');
 const injectHtmlParser2SpecialTags = require('./lib/markbind/src/patches/htmlparser2');
 const injectMarkdownItSpecialTags = require(
   './lib/markbind/src/lib/markdown-it-shared/markdown-it-escape-special-tags');
@@ -521,7 +522,7 @@ class Site {
       $('variable,span').each(function () {
         const name = $(this).attr('name') || $(this).attr('id');
         // Process the content of the variable with nunjucks, in case it refers to other variables.
-        const html = nunjucks.renderString($(this).html(), userDefinedVariables);
+        const html = nunjuckUtils.renderEscaped(nunjucks, $(this).html(), userDefinedVariables);
         userDefinedVariables[name] = html;
       });
     });

diff --git a/src/lib/markbind/src/parser.js b/src/lib/markbind/src/parser.js
@@ -7,6 +7,7 @@ const Promise = require('bluebird');
 const slugify = require('@sindresorhus/slugify');
 const componentParser = require('./parsers/componentParser');
 const componentPreprocessor = require('./preprocessors/componentPreprocessor');
+const nunjuckUtils = require('./utils/nunjuckUtils');
 
 const _ = {};
 _.clone = require('lodash/clone');
@@ -100,7 +101,7 @@ class Parser {
         return;
       }
       if (!pageVariables[variableName]) {
-        const variableValue = nunjucks.renderString(md.renderInline(variableElement.html()), {
+        const variableValue = nunjuckUtils.renderEscaped(nunjucks, md.renderInline(variableElement.html()), {
           ...importedVariables, ...pageVariables, ...userDefinedVariables, ...includedVariables,
         });
         pageVariables[variableName] = variableValue;
@@ -145,9 +146,13 @@ class Parser {
     // Extract page variables from the CHILD file
     const pageVariables
       = this.extractPageVariables(asIfAt, fileContent, userDefinedVariables, includeVariables);
-    const content = nunjucks.renderString(fileContent,
-                                          { ...pageVariables, ...includeVariables, ...userDefinedVariables },
-                                          { path: filePath });
+    const content = nunjuckUtils.renderEscaped(nunjucks, fileContent, {
+      ...pageVariables,
+      ...includeVariables,
+      ...userDefinedVariables,
+    }, {
+      path: filePath,
+    });
     const childContext = _.cloneDeep(context);
     childContext.cwf = asIfAt;
     childContext.variables = includeVariables;
@@ -180,8 +185,11 @@ class Parser {
         = this._renderIncludeFile(filePath, node, context, config);
       this.extractInnerVariablesIfNotProcessed(renderedContent, childContext, config, filePath);
       const innerVariables = this.getImportedVariableMap(filePath);
+
       Parser.VARIABLE_LOOKUP.get(filePath).forEach((value, variableName, map) => {
-        map.set(variableName, nunjucks.renderString(value, { ...userDefinedVariables, ...innerVariables }));
+        map.set(variableName, nunjuckUtils.renderEscaped(nunjucks, value, {
+          ...userDefinedVariables, ...innerVariables,
+        }));
       });
     });
   }
@@ -391,14 +399,17 @@ class Parser {
           = urlUtils.calculateNewBaseUrls(file, config.rootPath, config.baseUrlMap);
         const userDefinedVariables = config.userDefinedVariablesMap[path.resolve(parent, relative)];
         const pageVariables = this.extractPageVariables(file, data, userDefinedVariables, {});
-        let fileContent
-          = nunjucks.renderString(
-            data,
-            { ...pageVariables, ...userDefinedVariables },
-            { path: actualFilePath });
+        let fileContent = nunjuckUtils.renderEscaped(nunjucks, data, {
+          ...pageVariables,
+          ...userDefinedVariables,
+        }, {
+          path: actualFilePath,
+        });
         this._extractInnerVariables(fileContent, context, config);
         const innerVariables = this.getImportedVariableMap(context.cwf);
-        fileContent = nunjucks.renderString(fileContent, { ...userDefinedVariables, ...innerVariables });
+        fileContent = nunjuckUtils.renderEscaped(nunjucks, fileContent, {
+          ...userDefinedVariables, ...innerVariables,
+        });
         const fileExt = utils.getExt(file);
         if (utils.isMarkdownFileExt(fileExt)) {
           context.source = 'md';
@@ -461,16 +472,16 @@ class Parser {
       const { additionalVariables } = config;
       const pageVariables = this.extractPageVariables(actualFilePath, pageData, userDefinedVariables, {});
 
-      let fileContent = nunjucks.renderString(pageData,
-                                              {
-                                                ...pageVariables,
-                                                ...userDefinedVariables,
-                                                ...additionalVariables,
-                                              },
-                                              { path: actualFilePath });
+      let fileContent = nunjuckUtils.renderEscaped(nunjucks, pageData, {
+        ...pageVariables,
+        ...userDefinedVariables,
+        ...additionalVariables,
+      }, {
+        path: actualFilePath,
+      });
       this._extractInnerVariables(fileContent, currentContext, config);
       const innerVariables = this.getImportedVariableMap(currentContext.cwf);
-      fileContent = nunjucks.renderString(fileContent, {
+      fileContent = nunjuckUtils.renderEscaped(nunjucks, fileContent, {
         ...userDefinedVariables,
         ...additionalVariables,
         ...innerVariables,
@@ -613,7 +624,7 @@ class Parser {
                                                             this.rootPath, this.baseUrlMap);
           if (currentBase && currentBase.relative !== newBaseUrl) {
             cheerio.prototype.options.xmlMode = false;
-            const rendered = nunjucks.renderString(cheerio.html(node.children), {
+            const rendered = nunjuckUtils.renderEscaped(nunjucks, cheerio.html(node.children), {
               // This is to prevent the nunjuck call from converting {{hostBaseUrl}} to an empty string
               // and let the hostBaseUrl value be injected later.
               hostBaseUrl: '{{hostBaseUrl}}',

diff --git a/src/lib/markbind/src/preprocessors/componentPreprocessor.js b/src/lib/markbind/src/preprocessors/componentPreprocessor.js
@@ -8,6 +8,7 @@ const CyclicReferenceError = require('../handlers/cyclicReferenceError.js');
 const md = require('../lib/markdown-it');
 const utils = require('../utils');
 const urlUtils = require('../utils/urls');
+const nunjuckUtils = require('../utils/nunjuckUtils');
 
 const _ = {};
 _.has = require('lodash/has');
@@ -200,7 +201,7 @@ function _rebaseReferenceForStaticIncludes(pageData, element, config) {
   const newBase = fileBase.relative;
   const newBaseUrl = `{{hostBaseUrl}}/${newBase}`;
 
-  return nunjucks.renderString(pageData, { baseUrl: newBaseUrl }, { path: filePath });
+  return nunjuckUtils.renderEscaped(nunjucks, pageData, { baseUrl: newBaseUrl }, { path: filePath });
 }
 
 function _deleteIncludeAttributes(node) {
@@ -317,7 +318,9 @@ function _preprocessInclude(node, context, config, parser) {
   parser.extractInnerVariablesIfNotProcessed(content, childContext, config, filePath);
 
   const innerVariables = parser.getImportedVariableMap(filePath);
-  const fileContent = nunjucks.renderString(content, { ...userDefinedVariables, ...innerVariables });
+  const fileContent = nunjuckUtils.renderEscaped(nunjucks, content, {
+    ...userDefinedVariables, ...innerVariables,
+  });
 
   _deleteIncludeAttributes(element);
 

diff --git a/src/lib/markbind/src/utils/nunjuckUtils.js b/src/lib/markbind/src/utils/nunjuckUtils.js
@@ -0,0 +1,14 @@
+const START_ESCAPE_STR = '{% raw %}';
+const END_ESCAPE_STR = '{% endraw %}';
+const REGEX = new RegExp('{% *raw *%}(.*?){% *endraw *%}', 'gs');
+
+function preEscapeRawTags(pageData) {
+  return pageData.replace(REGEX, `${START_ESCAPE_STR}$&${END_ESCAPE_STR}`);
+}
+
+module.exports = {
+  renderEscaped(nunjucks, pageData, variableMap = {}, options = {}) {
+    const escapedPage = preEscapeRawTags(pageData);
+    return nunjucks.renderString(escapedPage, variableMap, options);
+  },
+};
diff --git a/test/functional/test_site_special_tags/_markbind/plugins/testSpecialTag.js b/test/functional/test_site_special_tags/_markbind/plugins/testSpecialTag.js
@@ -1,5 +1,7 @@
 const cheerio = module.parent.require('cheerio');
 
+const ESCAPE_REGEX = new RegExp('{% *raw *%}(.*?){% *endraw *%}', 'gs');
+
 /*
  Simple test plugin that whitelists <testtag> as a special tag.
  If encountered, it wraps the text node inside with some indication text as to
@@ -20,8 +22,27 @@ function preRender(content) {
   return $.html();
 }
 
+/*
+  Tests that special tags like <mustache> which would contain a lot of mustache syntax
+  like {{ }}, we are able to replace them with !success!success success!success!
+  without interference from other dependencies
+*/
+function postRender(content) {
+  const $ = cheerio.load(content);
+  const escapedNunjucks = $('mustache');
+  escapedNunjucks.each((index, element) => {
+    const unwrappedText = $(element).text();
+    const unescapedText = unwrappedText.replace(ESCAPE_REGEX, 'raw$1endraw');
+    const transformedText = unescapedText.replace(/{/g, '!success').replace(/}/g, 'success!');
+    $(element).text(transformedText);
+  });
+
+  return $.html();
+}
+
 
 module.exports = {
   preRender,
-  getSpecialTags: () => ['testtag'],
+  postRender,
+  getSpecialTags: () => ['testtag', 'mustache'],
 };
diff --git a/test/functional/test_site_special_tags/expected/index.html b/test/functional/test_site_special_tags/expected/index.html
@@ -24,7 +24,7 @@
       <div id="content-wrapper">
         <h1 id="functional-test-for-htmlparser2-and-markdown-it-patches-for-special-tags">Functional test for htmlparser2 and markdown-it patches for special tags<a class="fa fa-anchor" href="#functional-test-for-htmlparser2-and-markdown-it-patches-for-special-tags"></a></h1>
         <h2 id="so-far-as-to-comply-with-the-commonmark-spec">So far as to comply with the commonmark spec<a class="fa fa-anchor" href="#so-far-as-to-comply-with-the-commonmark-spec"></a></h2>
-        <p>There should be no text between this and the next <code>&lt;hr&gt;</code> tag in the browser, since it is a <code>&lt;script&gt;</code> tag.<br> There should be an alert with the value of 2 as well.</p>
+        <p>There should be no text between this and the next <code>&lt;hr&gt;</code> tag in the browser, since it is a <code>&lt;script&gt;</code> tag.<br> There should be an alert with the value of 2 as well.<br></p>
         <script>
           let x = 1;
 
@@ -42,6 +42,14 @@ <h2 id="so-far-as-to-comply-with-the-commonmark-spec">So far as to comply with t
         <p>There should be text between this and the next <code>&lt;hr&gt;</code> tag, since it is a special tag. All text should appear in the browser window as a single line, save for the comment which the browser still interprets. (but will be in the
           expected output)
         </p>
+        <mustache>
+raw
+
+!success!success This should be enclosed in success string success!success!
+
+!success This should also be enclosed in success strings success!success!success!
+endraw
+</mustache>
         <testtag>!success
 
 <these>
@@ -72,7 +80,7 @@ <h2 id="so-far-as-to-comply-with-the-commonmark-spec">So far as to comply with t
         <hr>
         <p>This should pass the htmlparser2 patch but not the markdown-it patch as it violates commonmark.<br> All lines after the first <code>!success</code> wrapping text will be wrapped in a <code>&lt;p&gt;...&lt;/p&gt;</code> tag as it is parsed as a
           markdown paragraph.
-        </p>
+          <br></p>
         <div>
           <testtag>!success
 <p>let x = 2;</p>

diff --git a/test/functional/test_site_special_tags/index.md b/test/functional/test_site_special_tags/index.md
@@ -29,6 +29,15 @@ There should be text between this and the next `<hr>` tag, since it is a special
 All text should appear in the browser window as a single line,
 save for the comment which the browser still interprets. (but will be in the expected output)
 
+<mustache>
+{%raw%}
+
+{{ This should be enclosed in success string }}
+
+{ This should also be enclosed in success strings }}}
+{%endraw%}
+</mustache>
+
 <testtag>
 <these>
 some text

diff --git a/test/unit/nunjuckUtils.test.js b/test/unit/nunjuckUtils.test.js
@@ -0,0 +1,23 @@
+const nunjucks = require('nunjucks');
+const nunjuckUtils = require('../../src/lib/markbind/src/utils/nunjuckUtils');
+
+test('Escaping nunjucks raw tags', () => {
+  const escapedString = 'This is a content with escaped data {%raw%} CONTENT {%endraw%}';
+  const escapedContent = nunjuckUtils.renderEscaped(nunjucks, escapedString);
+
+  expect(escapedContent).toBe(escapedString);
+});
+
+test('Escaping nunjucks with new lines', () => {
+  const escapedString = 'This is a content with escaped data\n {%raw%} \nCONTENT\n {%endraw%}';
+  const escapedContent = nunjuckUtils.renderEscaped(nunjucks, escapedString);
+
+  expect(escapedContent).toBe(escapedString);
+});
+
+test('Escaping multiple nunjucks raw tags', () => {
+  const escapedString = 'Multiple escapes: {%raw%} first {%endraw%} {%raw%} second {%endraw%}';
+  const escapedContent = nunjuckUtils.renderEscaped(nunjucks, escapedString);
+
+  expect(escapedContent).toBe(escapedString);
+});