gh-155397: Raise InvalidFileException for malformed XML plists - #156177
gh-155397: Raise InvalidFileException for malformed XML plists#156177jaideeppyne wants to merge 1 commit into
Conversation
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.
|
All commit authors signed the Contributor License Agreement. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8a455f9a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except LookupError as e: | ||
| # An <?xml ... ?> declaration naming an encoding that Python's | ||
| # codec registry does not know raises LookupError from expat. | ||
| raise InvalidFileException(str(e)) from e |
There was a problem hiding this comment.
When a caller supplies a custom dict_type whose constructor or __setitem__ raises KeyError or IndexError, the exception propagates through ParseFile() and is converted here because both exceptions subclass LookupError; a custom file object's read() can be affected similarly. Consequently, even valid XML now raises a misleading InvalidFileException and hides an error originating in caller-provided code, unlike the binary parser and the behavior before this change. Only the plain codec-registry LookupError produced for an unknown declared encoding should be translated.
Useful? React with 👍 / 👎.
1 participant
plistlib.load()/loads()is documented to raiseplistlib.InvalidFileExceptionwhen a file cannot be parsed, but_PlistParser.parse()called expat'sParseFile()with no exception translation. Two classes of malformed XML plist leaked the underlying exception instead:xml.parsers.expat.ExpatError.<?xml ... ?>declaration naming an encoding unknown to Python's codec registry raisedLookupError.Neither is a subclass of
ValueError(InvalidFileException's base), so callers following the documented contract did not catch them. This translates both intoInvalidFileExceptionand adds regression tests.Fixes #155397.