Repository: Python-Markdown/markdown
Pull request #804 (2019-03-08 02:07:04)

Optimize HTML_RE from quadratic time to linear
The test change may not be the desired result, but the previous behavior only happened by accident, since `HTML_RE` was matching

    <c.length;e++)
        d+=b.charAt(a.indexOf(c.charAt(e)));
            document
                .getElementById("e116142240")
                .innerHTML="<a href=\"mailto:"+d+"\">

as an “HTML tag”.  If the intention was to pass through the contents of `<script>` with no processing, then there’s a separate bug here.

andersk: To elaborate on the separate bug, compare the old behavior:

```pycon
>>> markdown.markdown('<span></span><script>e<c</script>')
'<p><span></span><script>e<c</script></p>'
>>> markdown.markdown('<span></span><script>e>c</script>')
'<p><span></span><script>e&gt;c</script></p>'
>>> markdown.markdown('<span></span><script>e < c</script>')
'<p><span></span><script>e &lt; c</script></p>'
>>> markdown.markdown('<span></span><script>e > c</script>')
'<p><span></span><script>e &gt; c</script></p>'
```

to the new behavior:

```pycon
>>> markdown.markdown('<span></span><script>e<c</script>')
'<p><span></span><script>e&lt;c</script></p>'
>>> markdown.markdown('<span></span><script>e>c</script>')
'<p><span></span><script>e&gt;c</script></p>'
>>> markdown.markdown('<span></span><script>e < c</script>')
'<p><span></span><script>e &lt; c</script></p>'
>>> markdown.markdown('<span></span><script>e > c</script>')
'<p><span></span><script>e &gt; c</script></p>'
```
waylan: If you want to check correct behavior, you can always use Babelmark:

* [`<span></span><script>e<c</script>`](http://johnmacfarlane.net/babelmark2/?normalize=1&text=%3Cspan%3E%3C%2Fspan%3E%3Cscript%3Ee%3Cc%3C%2Fscript%3E)
* [`<span></span><script>e>c</script>`](http://johnmacfarlane.net/babelmark2/?normalize=1&text=%3Cspan%3E%3C%2Fspan%3E%3Cscript%3Ee%3Ec%3C%2Fscript%3E)
* [`<span></span><script>e < c</script>`](http://johnmacfarlane.net/babelmark2/?normalize=1&text=%3Cspan%3E%3C%2Fspan%3E%3Cscript%3Ee+%3C+c%3C%2Fscript%3E)
* [`<span></span><script>e > c</script>`](http://johnmacfarlane.net/babelmark2/?normalize=1&text=%3Cspan%3E%3C%2Fspan%3E%3Cscript%3Ee+%3E+c%3C%2Fscript%3E)

Some things to note:

1. The reference implementation is markdown.pl.
2. There are two versions of the reference implementation, which usually agree. Sometimes the newer version fixes a bug and sometimes the newer version introduced a new bug.
3. If there is a disagreement between Python-Markdown and the reference implementation, we usually most closely match PHP Markdown, which is a very good (faithful to the rules) old-school implementation.


Based on the above, it seems that the current behavior is correct in most of the examples, but not all. And the changes you made actually cause the parser to deviate further from the correct behavior. Of course, these are pretty abstract examples, so it's not very clear how that translates to real-world use. 

waylan: For a basic overview, the [rules][1] specifically define when raw HTML is considered block-level and when it is not. Anything that does not match the block-level rules is considered to be span-level (even if the HTML spec disagrees) and is handled by the inline html pattern. As Markdown is parsed between span-level tags, but not within the tag itself, we simply identify anything between the opening (`<`) and closing (`<`) brackets as a tag.

However, you can also have individual characters as `<` or `>` and those are left as-is. So the regex has some understanding of a tag's syntax to work out the different between a complex tag and random independent brackets which are not part of a tag. That is also why comments are special cased. The contents of a comment are always ignored.

[1]: https://daringfireball.net/projects/markdown/syntax#html
waylan: So my initial thought was to reject this change as it deviates from the reference implementation.

And then there is the question of why we are converting the content of a `<script>` tag to an HTML entity. However, in the example in the test, that script tag is not considered block-level as it is not surrounded by blank lines. Looking back at the original report (#242), the appropriate response should have been to instruct the document author to make the script tag a proper block-level element as per the Markdown rules. So, in the end, we shouldn't care if we muck up the content of *inline* script tags.

Additionally, the existing behavior of the second and forth examples (independent `>`) actually breaks browser rendering. Converting them to HTML entities fixes that issue.

Given the above, I'm willing to accept this **if the tests are properly updated**. Let's remove the misleading `misc/escaped_chars_in_js` test and create some new tests based on the examples above. However, to make things more clear, lets not wrap them in script tags. Spans would more clearly communicate that this is testing how independent angle brackets behave within inline HTML (and nothing more).

```md
<span>e<c</span>
<span>e>c</span>
<span>e < c</span>
<span>e > c</span>
```

Finally, I would like to point out that all new tests (and changed tests) should be refactored for the [new testing framework][1] rather than the [legacy testing framework][2]. In other words, a new file should be created at `tests/test_syntax/inline/test_raw_html.py` which contains `markdown.test_tools.TestCase` unittests.

[1]: https://python-markdown.github.io/test_tools/#markdowntest_toolstestcase
[2]: https://python-markdown.github.io/test_tools/#markdowntest_toolslegacytestcase
andersk: Updated as requested.
waylan: Thanks for the update. Unfortunately our CI system is broken. We'll get that fixed first, then do a final review when we can confirm all tests pass.
andersk: Rebased onto 3.1.1 which has working CI.

Patch:
diff --git a/markdown/inlinepatterns.py b/markdown/inlinepatterns.py
index 74cda18..23f7c5a 100644
--- a/markdown/inlinepatterns.py
+++ b/markdown/inlinepatterns.py
@@ -153,7 +153,7 @@ AUTOLINK_RE = r'<((?:[Ff]|[Hh][Tt])[Tt][Pp][Ss]?://[^<>]*)>'
 AUTOMAIL_RE = r'<([^<> !]*@[^@<> ]*)>'
 
 # <...>
-HTML_RE = r'(\<([a-zA-Z/][^\>]*?|\!--.*?--)\>)'
+HTML_RE = r'(<([a-zA-Z/][^<>]*|!--(?:(?!<!--|-->).)*--)>)'
 
 # "&#38;" (decimal) or "&#x26;" (hex) or "&amp;" (named)
 ENTITY_RE = r'(&(?:\#[0-9]+|\#x[0-9a-fA-F]+|[a-zA-Z0-9]+);)'


Test patch:
diff --git a/tests/misc/escaped_chars_in_js.html b/tests/misc/escaped_chars_in_js.html
deleted file mode 100644
index b821fb1..0000000
--- a/tests/misc/escaped_chars_in_js.html
+++ /dev/null
@@ -1,12 +0,0 @@
-<p><span id="e116142240">[javascript protected email address]</span>
-<script type="text/javascript">
-    var a="gqMjyw7lZCaKk6p0J3uAUYS1.dbIW2hXzDHmiVNotOPRe_Ev@c4Gs58+LBr-F9QTfxn";
-    var b=a.split("").sort().join("");
-    var c="F_-F6F_-FMe_";
-    var d="";
-    for(var e=0;e<c.length;e++)
-    d+=b.charAt(a.indexOf(c.charAt(e)));
-        document
-            .getElementById("e116142240")
-            .innerHTML="<a href=\"mailto:"+d+"\">"+d+"</a>";
-</script></p>
\ No newline at end of file
diff --git a/tests/misc/escaped_chars_in_js.txt b/tests/misc/escaped_chars_in_js.txt
deleted file mode 100644
index ca917c8..0000000
--- a/tests/misc/escaped_chars_in_js.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-<span id="e116142240">[javascript protected email address]</span>
-<script type="text/javascript">
-    var a="gqMjyw7lZCaKk6p0J3uAUYS1.dbIW2hXzDHmiVNotOPRe_Ev@c4Gs58+LBr-F9QTfxn";
-    var b=a.split("").sort().join("");
-    var c="F_-F6F_-FMe_";
-    var d="";
-    for(var e=0;e<c.length;e++)
-    d+=b.charAt(a.indexOf(c.charAt(e)));
-        document
-            .getElementById("e116142240")
-            .innerHTML="<a href=\"mailto:"+d+"\">"+d+"</a>";
-</script>
diff --git a/tests/test_syntax/inline/test_raw_html.py b/tests/test_syntax/inline/test_raw_html.py
new file mode 100644
index 0000000..23ce8e5
--- /dev/null
+++ b/tests/test_syntax/inline/test_raw_html.py
@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+"""
+Python Markdown
+
+A Python implementation of John Gruber's Markdown.
+
+Documentation: https://python-markdown.github.io/
+GitHub: https://github.com/Python-Markdown/markdown/
+PyPI: https://pypi.org/project/Markdown/
+
+Started by Manfred Stienstra (http://www.dwerg.net/).
+Maintained for a few years by Yuri Takhteyev (http://www.freewisdom.org).
+Currently maintained by Waylan Limberg (https://github.com/waylan),
+Dmitry Shachnev (https://github.com/mitya57) and Isaac Muse (https://github.com/facelessuser).
+
+Copyright 2007-2018 The Python Markdown Project (v. 1.7 and later)
+Copyright 2004, 2005, 2006 Yuri Takhteyev (v. 0.2-1.6b)
+Copyright 2004 Manfred Stienstra (the original version)
+
+License: BSD (see LICENSE.md for details).
+"""
+
+from markdown.test_tools import TestCase
+
+
+class TestRawHtml(TestCase):
+    def test_inline_html_angle_brackets(self):
+        self.assertMarkdownRenders("<span>e<c</span>", "<p><span>e&lt;c</span></p>")
+        self.assertMarkdownRenders("<span>e>c</span>", "<p><span>e&gt;c</span></p>")
+        self.assertMarkdownRenders("<span>e < c</span>", "<p><span>e &lt; c</span></p>")
+        self.assertMarkdownRenders("<span>e > c</span>", "<p><span>e &gt; c</span></p>")