gh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` by chgnrdv · Pull Request #104266 · python/cpython
…r` types Set `Py_TPFLAGS_DISALLOW_INSTANTIATION` and unset `Py_TPFLAGS_BASETYPE` flags on `Reader` and `Writer` types to prevent their instantiation and subtyping
chgnrdv
changed the title
Disallow instantiation and subtyping of
gh-104265 Disallow instantiation and subtyping of _csv.Reader and _csv.Writer_csv.Reader and _csv.Writer
Thanks! This needs a NEWS entry, preferrably with a reference to when the bug was introduced. I think this should be backported through to 3.10.
Also, please add tests. There's a dedicated test.support helper for this. Grep through the code base to find it and see how it's used.
@erlend-aasland, are you talking about assert_python_ok? Can't we just check if TypeError with proper message is raised on attempt to create instance/subtype?
Thanks! This needs a NEWS entry, preferrably with a reference to when the bug was introduced. I think this should be backported through to 3.10.
Ah, 3.10 is in security fix mode, so we can only backport to 3.11.
erlend-aasland
changed the title
gh-104265 Disallow instantiation and subtyping of
gh-104265 Disallow instantiation of _csv.Reader and _csv.Writer_csv.Reader and _csv.Writer
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested:
- Revert
Py_TPFLAGS_BASETYPEchanges - Add NEWS entry
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.
Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
Looks like 6a02b38 from #23224 introduced this issue. How can I reflect this in NEWS entry text? Should I specify an issue number?
Indeed it did.
You can reference GitHub issues/PRs with the :gh: Sphinx directive. Here's a suggestion to a NEWS entry:
Prevent possible crash by disallow instantiation of the :class:`!_csv.Reader`
and :class:`!_csv.Writer` types. The regression was introduced by :gh:`23224`
in Python 3.10. Patch by <your-name-here>.
Thanks for making the requested changes!
@erlend-aasland: please review the changes made to this pull request.
Thanks @chgnrdv for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
…ter` (pythonGH-104266) (cherry picked from commit 06c2a48) Co-authored-by: chgnrdv <52372310+chgnrdv@users.noreply.github.com>