gh-155397: Raise InvalidFileException for malformed XML plists in plistlib - #155398
Open
simitana wants to merge 1 commit into
Open
gh-155397: Raise InvalidFileException for malformed XML plists in plistlib#155398simitana wants to merge 1 commit into
simitana wants to merge 1 commit into
Conversation
…in plistlib _PlistParser.parse() called expat's ParseFile() with no exception translation, so two classes of malformed XML plist escaped as the underlying exception instead of the documented InvalidFileException: * 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 (this is what CIFuzz found in pythongh-152211). Neither exception type is a ValueError, so code written against the documented contract (catching InvalidFileException, or even just ValueError) did not catch them.
|
All commit authors signed the Contributor License Agreement. |
picnixz
reviewed
Aug 8, 2026
Comment on lines
+188
to
+196
| try: | ||
| self.parser.ParseFile(fileobj) | ||
| except (ExpatError, LookupError): | ||
| # gh-155397: ExpatError is raised for XML that is not | ||
| # well-formed, and LookupError for a <?xml ... ?> declaration | ||
| # naming an unknown encoding; neither is a ValueError, so it | ||
| # would otherwise escape uncaught instead of the documented | ||
| # InvalidFileException. | ||
| raise InvalidFileException() |
Member
There was a problem hiding this comment.
This doesn't propagate the error messages so it won't be helpful. There are other places in plistlib that raises unexpected exceptions and we still don't know what to do with it. I would suggest we first discuss that.
Member
There was a problem hiding this comment.
For now:
- remove the long comment, it doesn't indicate anything
- appropriately create an exception message for that
- test it
And also I would wait for the decision on the ci-fuzz issue first, though I think we should indeed raise InvalidFileException here as documented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_PlistParser.parse()called expat'sParseFile()with no exception translation, so two classes of malformed XML plist escaped as the underlying exception instead of the documentedInvalidFileException:xml.parsers.expat.ExpatError.<?xml ... ?>declaration naming an encoding unknown to Python's codec registry raisedLookupError(this is what CIFuzz found in CIFuzz fails on the 3.13 and 3.14 branches: plistlib parse and LookupError #152211).Neither exception type is a
ValueError, so code written against the documented contract (catchingInvalidFileException, or even justValueError) did not catch them.The fix wraps
ParseFile()and translates both intoInvalidFileException, the same way_BinaryPlistParser.parse()already does for its own format.Fixes gh-155397.