Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-104265 Disallow instantiation of_csv.Reader and_csv.Writer#104266

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
erlend-aasland merged 13 commits intopython:mainfromchgnrdv:_csv-make-Reader-Writer-types-non-instantiable
May 7, 2023
Merged

gh-104265 Disallow instantiation of_csv.Reader and_csv.Writer#104266

erlend-aasland merged 13 commits intopython:mainfromchgnrdv:_csv-make-Reader-Writer-types-non-instantiable
May 7, 2023

Conversation

@chgnrdv
Copy link
Contributor

@chgnrdvchgnrdv commentedMay 7, 2023•
edited by bedevere-bot
Loading

Fixes#104265

SetPy_TPFLAGS_DISALLOW_INSTANTIATION and unsetPy_TPFLAGS_BASETYPE flags onReader andWriter types to prevent their instantiation and subtyping

…r` typesSet `Py_TPFLAGS_DISALLOW_INSTANTIATION` and unset `Py_TPFLAGS_BASETYPE` flags on `Reader` and `Writer` types to prevent their instantiation and subtyping
@chgnrdvchgnrdv changed the titleDisallow instantiation and subtyping of_csv.Reader and_csv.Writergh-104265 Disallow instantiation and subtyping of_csv.Reader and_csv.WriterMay 7, 2023
@erlend-aasland
Copy link
Contributor

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.

chgnrdv and Eclips4 reacted with thumbs up emoji

@erlend-aasland
Copy link
Contributor

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
Copy link
ContributorAuthor

chgnrdv commentedMay 7, 2023•
edited
Loading

@erlend-aasland, are you talking aboutassert_python_ok? Can't we just check ifTypeError with proper message is raised on attempt to create instance/subtype?

@erlend-aasland
Copy link
Contributor

There's a disallow_instantiation helper in test.support.

@erlend-aaslanderlend-aasland added the needs backport to 3.11only security fixes labelMay 7, 2023
@erlend-aasland
Copy link
Contributor

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
Copy link
Contributor

Some history, the original issue for applyingPEP-384 to the _csv extension module:

A later (duplicate issue); the linked PRs were closed and incorporated intogh-23224

As you can see, both the Writer and the Reader class hadPy_TPFLAGS_BASETYPE when they were converted from static to heap types. Removing that flag is abackwards incompatible change, as third-party software may depend on that behaviour. Please revert that particular change.

@erlend-aaslanderlend-aasland changed the titlegh-104265 Disallow instantiation and subtyping of_csv.Reader and_csv.Writergh-104265 Disallow instantiation of_csv.Reader and_csv.WriterMay 7, 2023
Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Changes requested:

  • RevertPy_TPFLAGS_BASETYPE changes
  • Add NEWS entry

@bedevere-bot
Copy link

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 phraseI 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
Copy link
ContributorAuthor

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

@erlend-aasland
Copy link
Contributor

Looks like6a02b38 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
Copy link
ContributorAuthor

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@erlend-aasland
Copy link
Contributor

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

chgnrdv reacted with thumbs up emoji

@erlend-aaslanderlend-aasland merged commit06c2a48 intopython:mainMay 7, 2023
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-104278 is a backport of this pull request to the3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11only security fixes labelMay 7, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 7, 2023
…ter` (pythonGH-104266)(cherry picked from commit06c2a48)Co-authored-by: chgnrdv <52372310+chgnrdv@users.noreply.github.com>
@chgnrdv
Copy link
ContributorAuthor

@erlend-aasland, thank you for your patient review :)

erlend-aasland reacted with heart emoji

kumaraditya303 pushed a commit that referenced this pull requestMay 8, 2023
…iter` (GH-104266) (#104278)gh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (GH-104266)(cherry picked from commit06c2a48)Co-authored-by: chgnrdv <52372310+chgnrdv@users.noreply.github.com>
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull requestMay 8, 2023
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (47 commits)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)pythongh-103193: Improve `getattr_static` test coverage (python#104286)  Trim trailing whitespace and test on CI (python#104275)pythongh-102500: Remove mention of bytes shorthand (python#104281)pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)pythongh-104273: Remove redundant len() calls in argparse function (python#104274)pythongh-64660: Don't hardcode Argument Clinic return converter result variable name (python#104200)pythongh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (python#104266)pythonGH-102613: Improve performance of `pathlib.Path.rglob()` (pythonGH-104244)pythongh-103650: Fix perf maps address format (python#103651)pythonGH-89812: Churn `pathlib.Path` methods (pythonGH-104243)  ...
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

_csvReader andWriter types shouldn't be directly instantiable

4 participants

@chgnrdv@erlend-aasland@bedevere-bot@miss-islington
[8]ページ先頭
©2009-2025 Movatter.jp