◐ Shell
clean mode source ↗

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 chgnrdv changed the title Disallow instantiation and subtyping of _csv.Reader and _csv.Writer gh-104265 Disallow instantiation and subtyping of _csv.Reader and _csv.Writer

May 7, 2023

@erlend-aasland

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.

@erlend-aasland

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.

@chgnrdv

@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?

@erlend-aasland

There's a disallow_instantiation helper in test.support.

@erlend-aasland

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

@erlend-aasland erlend-aasland changed the title gh-104265 Disallow instantiation and subtyping of _csv.Reader and _csv.Writer gh-104265 Disallow instantiation of _csv.Reader and _csv.Writer

May 7, 2023

erlend-aasland

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_BASETYPE changes
  • Add NEWS entry

@bedevere-bot

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.

@chgnrdv

Looks like 6a02b38 from #23224 introduced this issue. How can I reflect this in NEWS entry text? Should I specify an issue number?

@erlend-aasland

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>.

@chgnrdv

I have made the requested changes; please review again

@bedevere-bot

Thanks for making the requested changes!

@erlend-aasland: please review the changes made to this pull request.

erlend-aasland

erlend-aasland

…ub.com:chgnrdv/cpython into _csv-make-Reader-Writer-types-non-instantiable

erlend-aasland

@erlend-aasland

Thank you so much for the report and bugfix, Radislav; good job!

@miss-islington

Thanks @chgnrdv for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

May 7, 2023
…ter` (pythonGH-104266)

(cherry picked from commit 06c2a48)

Co-authored-by: chgnrdv <52372310+chgnrdv@users.noreply.github.com>

@chgnrdv

jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request

May 8, 2023

carljm added a commit to carljm/cpython that referenced this pull request

May 9, 2023