Skip to content

Commit f8a455f

Browse files
committed
gh-155397: Raise InvalidFileException for malformed XML plists
plistlib.load()/loads() is documented to raise InvalidFileException when a file cannot be parsed, but _PlistParser.parse() called expat's ParseFile() with no exception translation. Two classes of malformed XML plist leaked the underlying exception instead: * XML that is not well-formed raised xml.parsers.expat.ExpatError. * An <?xml ... ?> declaration naming an encoding unknown to Python's codec registry raised LookupError. Neither is a subclass of ValueError (InvalidFileException's base), so callers following the documented contract did not catch them. Translate both into InvalidFileException and add regression tests.
1 parent 67f4d53 commit f8a455f

3 files changed

Lines changed: 24 additions & 2 deletions

File tree

Lib/plistlib.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
import os
6868
import re
6969
import struct
70-
from xml.parsers.expat import ParserCreate
70+
from xml.parsers.expat import ExpatError, ParserCreate
7171

7272

7373
PlistFormat = enum.Enum('PlistFormat', 'FMT_XML FMT_BINARY', module=__name__)
@@ -185,7 +185,14 @@ def parse(self, fileobj):
185185
self.parser.EndElementHandler = self.handle_end_element
186186
self.parser.CharacterDataHandler = self.handle_data
187187
self.parser.EntityDeclHandler = self.handle_entity_decl
188-
self.parser.ParseFile(fileobj)
188+
try:
189+
self.parser.ParseFile(fileobj)
190+
except ExpatError as e:
191+
raise InvalidFileException(str(e)) from e
192+
except LookupError as e:
193+
# An <?xml ... ?> declaration naming an encoding that Python's
194+
# codec registry does not know raises LookupError from expat.
195+
raise InvalidFileException(str(e)) from e
189196
return self.root
190197

191198
def handle_entity_decl(self, entity_name, is_parameter_entity, value, base, system_id, public_id, notation_name):

Lib/test/test_plistlib.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,20 @@ def test_load_invalid_file(self):
916916
with self.assertRaises(plistlib.InvalidFileException):
917917
plistlib.loads(b"these are not plist file contents")
918918

919+
def test_xml_plist_not_well_formed(self):
920+
# gh-155397: a not-well-formed XML plist must raise the documented
921+
# InvalidFileException, not a raw xml.parsers.expat.ExpatError.
922+
with self.assertRaises(plistlib.InvalidFileException):
923+
plistlib.loads(b"<plist><foo></bar></plist>", fmt=plistlib.FMT_XML)
924+
925+
def test_xml_plist_unknown_encoding(self):
926+
# gh-155397: an <?xml ... ?> declaration naming an encoding that
927+
# Python does not know must raise the documented InvalidFileException,
928+
# not a raw LookupError.
929+
data = b'<?xml version="1.0" encoding="BogusEncoding"?><plist></plist>'
930+
with self.assertRaises(plistlib.InvalidFileException):
931+
plistlib.loads(data, fmt=plistlib.FMT_XML)
932+
919933
def test_modified_uid_negative(self):
920934
neg_uid = UID(1)
921935
neg_uid.data = -1 # dodge the negative check in the constructor
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:func:`plistlib.load` and :func:`plistlib.loads` now raise :exc:`plistlib.InvalidFileException` for XML plists that are not well-formed or that declare an unknown encoding, instead of leaking the underlying :exc:`~xml.parsers.expat.ExpatError` or :exc:`LookupError`.

0 commit comments

Comments
 (0)