Skip to content

Commit 738bda2

Browse files
committed
gh-148427: Fix bare except in expatreader.external_entity_ref()
Change bare `except:` to `except Exception:` so that KeyboardInterrupt and SystemExit propagate instead of being silently swallowed during external entity parsing. Move entity stack cleanup into a `finally` clause so the parser state is restored on both success and error paths.
1 parent 3a7df63 commit 738bda2

File tree

3 files changed

+68
-5
lines changed

3 files changed

+68
-5
lines changed

Lib/test/test_sax.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,65 @@ def test_expat_entityresolver_default(self):
10551055
self.assertEqual(result.getvalue(), start +
10561056
b"<doc></doc>")
10571057

1058+
def test_external_entity_ref_keyboard_interrupt(self):
1059+
# gh-148427: KeyboardInterrupt must propagate, not be swallowed
1060+
class KBHandler(handler.ContentHandler):
1061+
def startElement(self, name, attrs):
1062+
if name == 'entity':
1063+
raise KeyboardInterrupt('test')
1064+
1065+
parser = create_parser()
1066+
parser.setFeature(feature_external_ges, True)
1067+
parser.setEntityResolver(self.TestEntityResolver())
1068+
parser.setContentHandler(KBHandler())
1069+
1070+
with self.assertRaises(KeyboardInterrupt):
1071+
parser.feed('<!DOCTYPE doc [\n')
1072+
parser.feed(' <!ENTITY test SYSTEM "whatever">\n')
1073+
parser.feed(']>\n')
1074+
parser.feed('<doc>&test;</doc>')
1075+
parser.close()
1076+
1077+
def test_external_entity_ref_system_exit(self):
1078+
# gh-148427: SystemExit must propagate, not be swallowed
1079+
class ExitHandler(handler.ContentHandler):
1080+
def startElement(self, name, attrs):
1081+
if name == 'entity':
1082+
raise SystemExit(42)
1083+
1084+
parser = create_parser()
1085+
parser.setFeature(feature_external_ges, True)
1086+
parser.setEntityResolver(self.TestEntityResolver())
1087+
parser.setContentHandler(ExitHandler())
1088+
1089+
with self.assertRaises(SystemExit):
1090+
parser.feed('<!DOCTYPE doc [\n')
1091+
parser.feed(' <!ENTITY test SYSTEM "whatever">\n')
1092+
parser.feed(']>\n')
1093+
parser.feed('<doc>&test;</doc>')
1094+
parser.close()
1095+
1096+
def test_external_entity_ref_stack_cleanup(self):
1097+
# gh-148427: _entity_stack must be cleaned up after errors
1098+
class ErrorHandler(handler.ContentHandler):
1099+
def startElement(self, name, attrs):
1100+
if name == 'entity':
1101+
raise ValueError('test error')
1102+
1103+
parser = create_parser()
1104+
parser.setFeature(feature_external_ges, True)
1105+
parser.setEntityResolver(self.TestEntityResolver())
1106+
parser.setContentHandler(ErrorHandler())
1107+
1108+
with self.assertRaises(SAXParseException):
1109+
parser.feed('<!DOCTYPE doc [\n')
1110+
parser.feed(' <!ENTITY test SYSTEM "whatever">\n')
1111+
parser.feed(']>\n')
1112+
parser.feed('<doc>&test;</doc>')
1113+
parser.close()
1114+
1115+
self.assertEqual(len(parser._entity_stack), 0)
1116+
10581117
# ===== Attributes support
10591118

10601119
class AttrGatherer(ContentHandler):

Lib/xml/sax/expatreader.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -424,11 +424,11 @@ def external_entity_ref(self, context, base, sysid, pubid):
424424

425425
try:
426426
xmlreader.IncrementalParser.parse(self, source)
427-
except:
428-
return 0 # FIXME: save error info here?
429-
430-
(self._parser, self._source) = self._entity_stack[-1]
431-
del self._entity_stack[-1]
427+
except Exception:
428+
return 0
429+
finally:
430+
(self._parser, self._source) = self._entity_stack[-1]
431+
del self._entity_stack[-1]
432432
return 1
433433

434434
def skipped_entity_handler(self, name, is_pe):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix bare ``except`` clause in :mod:`xml.sax` expatreader's external entity
2+
handler that silently swallowed :exc:`KeyboardInterrupt` and
3+
:exc:`SystemExit`. Also fix entity parser stack leak on errors by moving
4+
cleanup into a ``finally`` clause.

0 commit comments

Comments
 (0)