Avoid a pop from an empty list in HTMLToContentStateHandler (#5004)

CHANGELOG.txt

@@ -9,6 +9,7 @@ Changelog
  * Fix: Set `SERVER_PORT` to 443 in `Page.dummy_request()` for HTTPS sites (Sergey Fedoseev)
  * Fix: Include port number in `Host` header of `Page.dummy_request()` (Sergey Fedoseev)
  * Fix: Validation error messages in `InlinePanel` no longer count towards `max_num` when disabling the 'add' button (Todd Dembrey, Thibaud Colas)
+ * Fix: Rich text to contentstate conversion now ignores stray closing tags (frmdstryr)
 
 
 2.4 (19.12.2018)

CONTRIBUTORS.rst

@@ -338,6 +338,7 @@ Contributors
 * Maylon Pedroso
 * Thijs Walcarius
 * mukesh5
+* frmdstryr
 
 Translators
 ===========

docs/releases/2.5.rst

@@ -24,6 +24,7 @@ Bug fixes
  * Set ``SERVER_PORT`` to 443 in ``Page.dummy_request()`` for HTTPS sites (Sergey Fedoseev)
  * Include port number in ``Host`` header of ``Page.dummy_request()`` (Sergey Fedoseev)
  * Validation error messages in ``InlinePanel`` no longer count towards ``max_num`` when disabling the 'add' button (Todd Dembrey, Thibaud Colas)
+ * Rich text to contentstate conversion now ignores stray closing tags (frmdstryr)
 
 
 Upgrade considerations

wagtail/admin/rich_text/converters/html_to_contentstate.py

@@ -306,6 +306,8 @@ class HtmlToContentStateHandler(HTMLParser):
             element_handler.handle_starttag(name, attrs, self.state, self.contentstate)
 
     def handle_endtag(self, name):
+        if not self.open_elements:
+            return  # avoid a pop from an empty list if we have an extra end tag
         expected_name, element_handler = self.open_elements.pop()
         assert name == expected_name, "Unmatched tags: expected %s, got %s" % (expected_name, name)
         if element_handler:

wagtail/admin/tests/test_contentstate.py

@@ -753,3 +753,37 @@ class TestHtmlToContentState(TestCase):
                 {'inlineStyleRanges': [], 'text': 'Arthur "two sheds" Jackson <the third> & his wife', 'depth': 0, 'type': 'unstyled', 'key': '00000', 'entityRanges': []},
             ]
         })
+
+    def test_extra_end_tag_before(self):
+        converter = ContentstateConverter(features=[])
+        result = json.loads(converter.from_database_format(
+            '''
+            </p>
+            <p>Before</p>
+            '''
+        ))
+        # The leading </p> tag should be ignored instead of blowing up with a
+        # pop from empty list error
+        self.assertContentStateEqual(result, {
+            'entityMap': {},
+            'blocks': [
+                {'inlineStyleRanges': [], 'text': 'Before', 'depth': 0, 'type': 'unstyled', 'key': '00000', 'entityRanges': []},
+            ]
+        })
+
+    def test_extra_end_tag_after(self):
+        converter = ContentstateConverter(features=[])
+        result = json.loads(converter.from_database_format(
+            '''
+            <p>After</p>
+            </p>
+            '''
+        ))
+        # The tailing </p> tag should be ignored instead of blowing up with a
+        # pop from empty list error
+        self.assertContentStateEqual(result, {
+            'entityMap': {},
+            'blocks': [
+                {'inlineStyleRanges': [], 'text': 'After', 'depth': 0, 'type': 'unstyled', 'key': '00000', 'entityRanges': []},
+            ]
+        })