Fix parse() raising TypeError on text-mode file-like objects - #594
Open
afonsojanu wants to merge 1 commit into
Open
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
_open_resource() returned a text-mode file-like object's data untouched, but convert_to_utf8() always matches it against a bytes regex, so passing an io.StringIO ends up raising TypeError instead of parsing. The docstring for parse() even suggests wrapping untrusted strings in io.StringIO, which is exactly the case that breaks. This encodes the read data to utf-8 when it comes back as str, mirroring what already happens a few lines down for plain string input.
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
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.
Related to #427.
_open_resource()returns whatever a file-like object's.read()gives back untouched. Forio.BytesIOthat's bytes, which is fine, but forio.StringIOit's a plainstr, andconvert_to_utf8()always matches its input against a bytes regex (RE_XML_PI_ENCODING), so it blows up with:The
parse()docstring actually recommends wrapping an untrusted string inio.StringIOorio.BytesIO, so this hits anyone following that advice with the text variant. Passing a plainstrdirectly already works, since that path already encodes to utf-8 a few lines further down — this just applies the same treatment when the data comes from a text-mode file-like object.Fix is a small change in
_open_resource(): if the object we read from a file-like isn't bytes, encode it, matching the existing plain-string branch. Added two tests: one directly on_open_resource(), one going throughparse()with anio.StringIO. Confirmed both fail with the originalTypeErrorwithout this change and pass with it; full existing suite (4335 tests) still passes.Targeting
releasesrather thanmainsince this is the 6.0.x maintenance branch where the bug actually lives — I noticedmainalready handles this correctly after the_to_in_memory_file()rework, so this is just a small backport for people on the current stable line.