Merge pull request #1460 from andersk/inline-text-quadratic

Improve worst-case performance of inline.text regex
markedjs · Apr 5, 2019 · b1ddd3c · b1ddd3c
2 parents ba1de1e + be27472
commit b1ddd3c
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 8 deletions.
diff --git a/lib/marked.js b/lib/marked.js
@@ -546,7 +546,7 @@ var inline = {
   code: /^(`+)([^`]|[^`][\s\S]*?[^`])\1(?!`)/,
   br: /^( {2,}|\\)\n(?!\s*$)/,
   del: noop,
-  text: /^(`+|[^`])[\s\S]*?(?=[\\<!\[`*]|\b_| {2,}\n|$)/
+  text: /^(`+|[^`])(?:[\s\S]*?(?:(?=[\\<!\[`*]|\b_|$)|[^ ](?= {2,}\n))|(?= {2,}\n))/
 };
 
 // list of punctuation marks from common mark spec
@@ -615,10 +615,7 @@ inline.gfm = merge({}, inline.normal, {
   url: /^((?:ftp|https?):\/\/|www\.)(?:[a-zA-Z0-9\-]+\.?)+[^\s<]*|^email/,
   _backpedal: /(?:[^?!.,:;*_~()&]+|\([^)]*\)|&(?![a-zA-Z0-9]+;$)|[?!.,:;*_~)]+(?!$))+/,
   del: /^~+(?=\S)([\s\S]*?\S)~+/,
-  text: edit(inline.text)
-    .replace(']|', '~]|')
-    .replace('|$', '|https?://|ftp://|www\\.|[a-zA-Z0-9.!#$%&\'*+/=?^_`{\\|}~-]+@|$')
-    .getRegex()
+  text: /^(`+|[^`])(?:[\s\S]*?(?:(?=[\\<!\[`*~]|\b_|https?:\/\/|ftp:\/\/|www\.|$)|[^ ](?= {2,}\n)|[^a-zA-Z0-9.!#$%&'*+\/=?_`{\|}~-](?=[a-zA-Z0-9.!#$%&'*+\/=?_`{\|}~-]+@))|(?= {2,}\n|[a-zA-Z0-9.!#$%&'*+\/=?_`{\|}~-]+@))/
 });
 
 inline.gfm.url = edit(inline.gfm.url, 'i')
@@ -630,7 +627,7 @@ inline.gfm.url = edit(inline.gfm.url, 'i')
 
 inline.breaks = merge({}, inline.gfm, {
   br: edit(inline.br).replace('{2,}', '*').getRegex(),
-  text: edit(inline.gfm.text).replace('{2,}', '*').getRegex()
+  text: edit(inline.gfm.text).replace(/\{2,\}/g, '*').getRegex()
 });
 
 /**

diff --git a/test/redos/quadratic_br.js b/test/redos/quadratic_br.js
@@ -0,0 +1,4 @@
+module.exports = {
+  markdown: `a${' '.repeat(50000)}`,
+  html: `<p>a${' '.repeat(50000)}</p>`
+};
diff --git a/test/redos/quadratic_email.js b/test/redos/quadratic_email.js
@@ -0,0 +1,4 @@
+module.exports = {
+  markdown: 'a'.repeat(50000),
+  html: `<p>${'a'.repeat(50000)}</p>`
+};
diff --git a/test/specs/gfm/gfm.0.28.json b/test/specs/gfm/gfm.0.28.json
@@ -141,8 +141,7 @@
     "section": "Autolinks",
     "html": "<p><a href=\"mailto:a.b-c_d@a.b\">a.b-c_d@a.b</a></p>\n<p><a href=\"mailto:a.b-c_d@a.b\">a.b-c_d@a.b</a>.</p>\n<p>a.b-c_d@a.b-</p>\n<p>a.b-c_d@a.b_</p>",
     "markdown": "a.b-c_d@a.b\n\na.b-c_d@a.b.\n\na.b-c_d@a.b-\n\na.b-c_d@a.b_",
-    "example": 607,
-    "shouldFail": true
+    "example": 607
   },
   {
     "section": "Disallowed Raw HTML",

diff --git a/test/specs/redos-spec.js b/test/specs/redos-spec.js
@@ -0,0 +1,24 @@
+const path = require('path');
+const fs = require('fs');
+
+const redosDir = path.resolve(__dirname, '../redos');
+
+describe('ReDOS tests', () => {
+  const files = fs.readdirSync(redosDir);
+  files.forEach(file => {
+    if (!file.match(/\.js$/)) {
+      return;
+    }
+
+    it(file, () => {
+      const spec = require(path.resolve(redosDir, file));
+      const before = process.hrtime();
+      expect(spec).toRender(spec.html);
+      const elapsed = process.hrtime(before);
+      if (elapsed[0] > 0) {
+        const s = (elapsed[0] + elapsed[1] * 1e-9).toFixed(3);
+        fail(`took too long: ${s}s`);
+      }
+    });
+  });
+});