fix: allow DOCTYPE with disable_entities=True (default)

Simplify disable_entities overly defensive parser config.

Fixes #397.

Author: martinblech

tests/test_xmltodict.py

@@ -419,7 +419,7 @@ def test_disable_entities_true_rejects_xmlbomb():
     ]>
     <bomb>&c;</bomb>
     """
-    with pytest.raises(expat.ExpatError, match="entities are disabled"):
+    with pytest.raises(ValueError, match="entities are disabled"):
         parse(xml, disable_entities=True)
 
 
@@ -437,39 +437,57 @@ def test_disable_entities_false_returns_xmlbomb():
     assert parse(xml, disable_entities=False) == expectedResult
 
 
-def test_disable_entities_true_rejects_external_dtd():
+def test_external_entity():
     xml = """
     <!DOCTYPE external [
         <!ENTITY ee SYSTEM "http://www.python.org/">
     ]>
     <root>&ee;</root>
     """
-    with pytest.raises(expat.ExpatError, match="entities are disabled"):
-        parse(xml, disable_entities=True)
+    with pytest.raises(ValueError, match="entities are disabled"):
+        parse(xml)
+    assert parse(xml, disable_entities=False) == {"root": None}
 
 
-def test_disable_entities_true_attempts_external_dtd():
+def test_external_entity_with_custom_expat():
     xml = """
     <!DOCTYPE external [
         <!ENTITY ee SYSTEM "http://www.python.org/">
     ]>
     <root>&ee;</root>
     """
 
-    def raising_external_ref_handler(*args, **kwargs):
-        parser = ParserCreate(*args, **kwargs)
-        parser.ExternalEntityRefHandler = lambda *x: 0
-        return parser
-    expat.ParserCreate = raising_external_ref_handler
-    # Using this try/catch because a TypeError is thrown before
-    # the ExpatError.
-    try:
-        parse(xml, disable_entities=False, expat=expat)
-    except expat.ExpatError:
-        assert True
-    else:
-        assert False
-    expat.ParserCreate = ParserCreate
+    class CustomExpat:
+        def __init__(self, external_entity_result):
+            self.external_entity_result = external_entity_result
+
+        def ParserCreate(self, *args, **kwargs):
+            parser = ParserCreate(*args, **kwargs)
+
+            def _handler(*args, **kwargs):
+                return self.external_entity_result
+
+            parser.ExternalEntityRefHandler = _handler
+            return parser
+
+        ExpatError = expat.ExpatError
+
+    with pytest.raises(expat.ExpatError):
+        parse(xml, disable_entities=False, expat=CustomExpat(0))
+    assert parse(xml, disable_entities=False, expat=CustomExpat(1)) == {"root": None}
+    with pytest.raises(ValueError):
+        assert parse(xml, disable_entities=True, expat=CustomExpat(1))
+    with pytest.raises(ValueError):
+        assert parse(xml, disable_entities=True, expat=CustomExpat(0))
+
+
+def test_disable_entities_true_allows_doctype_without_entities():
+    xml = """<?xml version='1.0' encoding='UTF-8'?>
+    <!DOCTYPE data SYSTEM "diagram.dtd">
+    <foo>bar</foo>
+    """
+    assert parse(xml, disable_entities=True) == {"foo": "bar"}
+    assert parse(xml, disable_entities=False) == {"foo": "bar"}
 
 
 def test_disable_entities_allows_comments_by_default():

xmltodict.py

@@ -355,22 +355,9 @@ def parse(xml_input, encoding=None, expat=expat, process_namespaces=False,
     parser.buffer_text = True
     if disable_entities:
         def _forbid_entities(*_args, **_kwargs):
-            raise expat.ExpatError("xmltodict.parse(): entities are disabled")
-
-        def _forbid_entities_default(text):
-            if not text:
-                return
-            stripped = text.lstrip()
-            if stripped.startswith('<!--'):
-                return
-            if stripped.startswith('<!') or stripped.startswith('&'):
-                _forbid_entities()
-
-        # Reject DTD/entity constructs explicitly instead of ignoring them.
-        parser.DefaultHandler = _forbid_entities_default
+            raise ValueError("entities are disabled")
+
         parser.EntityDeclHandler = _forbid_entities
-        parser.StartDoctypeDeclHandler = _forbid_entities
-        parser.ExternalEntityRefHandler = _forbid_entities
     if hasattr(xml_input, 'read'):
         parser.ParseFile(xml_input)
     elif isgenerator(xml_input):