Repository: zestedesavoir/zmarkdown
Pull request #325 (2019-05-08 09:22:44)

zmarkdown: Resolves conflicts between remark-ping, rehype-katex and rehype-sanitize
# Context

On the last stable version of Zeste de Savoir, a bug was reported on ZMarkdown as issue #324 , concerning a bug with remark-ping; some classes in the formatted HTML were missing. The issue is due to a bad configuration of the rehype-sanitize module.

At the same time, a bug with math was reported, and was first believed to be a dependency problem, while it was only rehype-sanitize oversanitizing KaTeX HTML too; this pull request should then also resolve the problem which led to issue #323 , although another step to resolving this issue may be taken later (see below).

# Resolution

To solve these bugs, it was needed to change the rehype-sanitize configuration to:

- allow classes on span elements for both ping and math;
- allow inline styles on span elements for math;
- allow every MathML element to pass through for KaTeX MathML variant.

Also, two tests were written in a new `sanitizer.tests.js` file to check problems with the sanitizer; composed of only two tests (math and ping) today, this file could add more checks for different HTML tags used by various plugins.

# What remains?

- Merging this PR would solve ZMarkdown bugs in production.
- Before doing it, I need someone to check my ping test, which does not fail correctly for some reasons (related with Jest, I think); to test it, please make the ping test fail, and see what happens (tests pass but with Jest error).
- On issue #324 , it will be solved entirely by this pull request.
- On issue #323 , the problem which led to is resolved by the PR, but pinning the dependency somewhere as @artragis suggested could be a wise idea to avoid future conflicts with versions between zds-site and zmarkdown.


Patch:
diff --git a/packages/zmarkdown/config/remark.js b/packages/zmarkdown/config/remark.js
index 7f8db1ee..569d9ec0 100644
--- a/packages/zmarkdown/config/remark.js
+++ b/packages/zmarkdown/config/remark.js
@@ -21,11 +21,60 @@ const gh = require('hast-util-sanitize/lib/github')
 const merge = require('deepmerge')
 
 const sanitizeConfig = merge(gh, {
-  tagNames: ['span', 'abbr', 'figure', 'figcaption', 'iframe'],
+  tagNames: [
+    'span',
+    'abbr',
+    'figure',
+    'figcaption',
+    'iframe',
+    // MathML elements
+    'math',
+    'maction',
+    'maligngroup',
+    'malignmark',
+    'menclose',
+    'merror',
+    'mfenced',
+    'mfrac',
+    'mglyph',
+    'mi',
+    'mlabeledtr',
+    'mlongdiv',
+    'mmultiscripts',
+    'mn',
+    'mo',
+    'mover',
+    'mpadded',
+    'mphantom',
+    'mroot',
+    'mrow',
+    'ms',
+    'mscarries',
+    'mscarry',
+    'msgroup',
+    'mstack',
+    'msline',
+    'mspace',
+    'msqrt',
+    'msrow',
+    'mstyle',
+    'msub',
+    'msubsup',
+    'msup',
+    'mtable',
+    'mtd',
+    'mtext',
+    'mtr',
+    'munder',
+    'munderover',
+    'semantics',
+    'annotation',
+    'annotation-xml',
+  ],
   attributes: {
     a: ['ariaHidden', 'class', 'className'],
     div: ['id', 'class', 'className'],
-    span: ['id', 'className'],
+    span: ['id', 'className', 'class', 'style'],
     h1: ['ariaHidden'],
     h2: ['ariaHidden'],
     h3: ['ariaHidden'],


Test patch:
diff --git a/packages/zmarkdown/__tests__/__snapshots__/sanitizer.tests.js.snap b/packages/zmarkdown/__tests__/__snapshots__/sanitizer.tests.js.snap
new file mode 100644
index 00000000..cc30e646
--- /dev/null
+++ b/packages/zmarkdown/__tests__/__snapshots__/sanitizer.tests.js.snap
@@ -0,0 +1,5 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`#math do not oversanitize math - test frac 1`] = `"<p><span class=\\"inlineMath inlineMathDouble\\"><span class=\\"katex-display\\"><span class=\\"katex\\"><span class=\\"katex-mathml\\"><math><semantics><mrow><mi>r</mi><mi>a</mi><mi>c</mi><mrow><mn>1</mn><mo>+</mo><mn>1</mn></mrow><mrow><mi>x</mi><mo>+</mo><mi>y</mi></mrow></mrow><annotation>rac{1+1}{x+y}</annotation></semantics></math></span><span class=\\"katex-html\\"><span class=\\"base\\"><span class=\\"strut\\" style=\\"height:0.8388800000000001em;vertical-align:-0.19444em;\\"></span><span class=\\"mord mathdefault\\" style=\\"margin-right:0.02778em;\\">r</span><span class=\\"mord mathdefault\\">a</span><span class=\\"mord mathdefault\\">c</span><span class=\\"mord\\"><span class=\\"mord\\">1</span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span><span class=\\"mbin\\">+</span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span><span class=\\"mord\\">1</span></span><span class=\\"mord\\"><span class=\\"mord mathdefault\\">x</span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span><span class=\\"mbin\\">+</span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span><span class=\\"mord mathdefault\\" style=\\"margin-right:0.03588em;\\">y</span></span></span></span></span></span></span></p>"`;
+
+exports[`#math do not oversanitize math - test sub/sup 1`] = `"<p><span class=\\"inlineMath inlineMathDouble\\"><span class=\\"katex-display\\"><span class=\\"katex\\"><span class=\\"katex-mathml\\"><math><semantics><mrow><msub><mn>1</mn><mi>n</mi></msub><mo>+</mo><msup><mn>1</mn><mi>n</mi></msup></mrow><annotation>1_n + 1^n</annotation></semantics></math></span><span class=\\"katex-html\\"><span class=\\"base\\"><span class=\\"strut\\" style=\\"height:0.79444em;vertical-align:-0.15em;\\"></span><span class=\\"mord\\"><span class=\\"mord\\">1</span><span class=\\"msupsub\\"><span class=\\"vlist-t vlist-t2\\"><span class=\\"vlist-r\\"><span class=\\"vlist\\" style=\\"height:0.151392em;\\"><span style=\\"top:-2.5500000000000003em;margin-left:0em;margin-right:0.05em;\\"><span class=\\"pstrut\\" style=\\"height:2.7em;\\"></span><span class=\\"sizing reset-size6 size3 mtight\\"><span class=\\"mord mathdefault mtight\\">n</span></span></span></span><span class=\\"vlist-s\\">​</span></span><span class=\\"vlist-r\\"><span class=\\"vlist\\" style=\\"height:0.15em;\\"><span></span></span></span></span></span></span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span><span class=\\"mbin\\">+</span><span class=\\"mspace\\" style=\\"margin-right:0.2222222222222222em;\\"></span></span><span class=\\"base\\"><span class=\\"strut\\" style=\\"height:0.7143919999999999em;vertical-align:0em;\\"></span><span class=\\"mord\\"><span class=\\"mord\\">1</span><span class=\\"msupsub\\"><span class=\\"vlist-t\\"><span class=\\"vlist-r\\"><span class=\\"vlist\\" style=\\"height:0.7143919999999999em;\\"><span style=\\"top:-3.113em;margin-right:0.05em;\\"><span class=\\"pstrut\\" style=\\"height:2.7em;\\"></span><span class=\\"sizing reset-size6 size3 mtight\\"><span class=\\"mord mathdefault mtight\\">n</span></span></span></span></span></span></span></span></span></span></span></span></span></p>"`;
diff --git a/packages/zmarkdown/__tests__/sanitizer.tests.js b/packages/zmarkdown/__tests__/sanitizer.tests.js
new file mode 100644
index 00000000..2857c4bf
--- /dev/null
+++ b/packages/zmarkdown/__tests__/sanitizer.tests.js
@@ -0,0 +1,33 @@
+const remarkConfig = require('../config/remark')
+const rebberConfig = require('../config/rebber')
+
+// Disable typographical corrections for easier testing
+remarkConfig.noTypography = true
+remarkConfig._test = true
+
+const zmarkdown = require('../server')
+
+const renderString = (config = {remarkConfig, rebberConfig}) =>
+  (input) =>
+    zmarkdown(config).renderString(input).then((vfile) =>
+      vfile.toString().trim())
+
+describe('#ping', () => {
+  it('do not oversanitize ping', () => {
+    const rendered = renderString()('@Clem')
+    expect(rendered).resolves.toContain('class="ping ping-link"')
+    expect(rendered).resolves.toContain('class="ping-username"')
+  })
+})
+
+describe('#math', () => {
+  it('do not oversanitize math - test sub/sup', () => {
+    const rendered = renderString()('$$1_n + 1^n$$')
+    expect(rendered).resolves.toMatchSnapshot()
+  })
+
+  it('do not oversanitize math - test frac', () => {
+    const rendered = renderString()('$$\frac{1+1}{x+y}$$')
+    expect(rendered).resolves.toMatchSnapshot()
+  })
+})