Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-117722: Fix Stream.readuntil with non-bytes buffer objects #117723

Merged
merged 2 commits into from Apr 11, 2024

Conversation

bmerry
Copy link
Contributor

@bmerry bmerry commented Apr 10, 2024

PR #16429 introduced support for an iterable of separators in Stream.readuntil. Since bytes-like types are themselves iterable, this can introduce ambiguities in deciding whether the argument is an iterator of separators or a singleton separator. In #16429, only 'bytes' was considered a singleton, but this will break code that passes other buffer object types.

Fix it by only supporting tuples rather than arbitrary iterables.

Closes #117722.


📚 Documentation preview 📚: https://cpython-previews--117723.org.readthedocs.build/

PR python#16429 introduced support for an iterable of separators in
Stream.readuntil. Since bytes-like types are themselves iterable, this
can introduce ambiguities in deciding whether the argument is an
iterator of separators or a singleton separator. In python#16429, only 'bytes'
was considered a singleton, but this will break code that passes other
buffer object types.

Fix it by only supporting tuples rather than arbitrary iterables.

Closes python#117722.
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Since your previous change made it into 3.13a6 it will be somewhat interesting to see if anybody actually notices a problem with that -- which would be an indicator of how much this matters (I expect very little :-). (If it hadn't made it in, I'd have recommended merging the changelog snippets, but it's too late for that now.)

I noticed that there isn't yet a bullet for this in Doc/whatsnew/3.13.rst -- do you feel like adding one? If you do, you could add it to this PR; I'll keep it open for a day or until I hear from you.

@bmerry
Copy link
Contributor Author

bmerry commented Apr 11, 2024

I noticed that there isn't yet a bullet for this in Doc/whatsnew/3.13.rst -- do you feel like adding one?

Sure - done.

@gvanrossum gvanrossum merged commit 01a51f9 into python:main Apr 11, 2024
33 checks passed
@gvanrossum
Copy link
Member

Thanks! You're welcome to contribute more any time, it's been a pleasure to work with you. (And again, apologies for the delay. :-)

@Mohsin1910
Copy link

Mohsin1910 commented Apr 11, 2024 via email

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
…ython#117723)

pythongh-16429 introduced support for an iterable of separators in
Stream.readuntil. Since bytes-like types are themselves iterable, this
can introduce ambiguities in deciding whether the argument is an
iterator of separators or a singleton separator. In pythongh-16429, only 'bytes'
was considered a singleton, but this will break code that passes other
buffer object types.

Fix it by only supporting tuples rather than arbitrary iterables.

Closes pythongh-117722.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.13.0a6 breaks asyncio.Stream.readuntil with bytearray separator
3 participants