Skip to content

gh-50644: Forbid pickling of codecs streams #109180

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 10, 2023
Merged

gh-50644: Forbid pickling of codecs streams #109180

merged 2 commits into from
Sep 10, 2023

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 9, 2023 •
edited
Loading

Attempts to pickle or create a shallow or deep copy of codecs streams now raise a TypeError.

Previously, copying failed with a RecursionError, while pickling produced wrong results that eventually caused unpickling to fail with a RecursionError.

@serhiy-storchaka
pythongh-50644: Forbid pickling of codecs streams
6f83e12 
Attempts to pickle or create a shallow or deep copy of codecs streams
now raise a TypeError.

Previously, stream pickling produced invalid data, which attempts to read
resulted in a RecursionError, as well as attempts to create a copy of
the stream.
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Sep 9, 2023
@bedevere-bot bedevere-bot mentioned this pull request Apr 10, 2022
Closed
malemburg
malemburg approved these changes Sep 9, 2023
View reviewed changes
Copy link
Member

@malemburg malemburg left a comment

Choose a reason for hiding this comment

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

Other than the minor nit with the news entry this LGTM. Thanks, Serhiy.

Misc/NEWS.d/next/Library/2023-09-09-15-08-37.gh-issue-50644.JUAZOh.rst Outdated
Attempts to pickle or create a shallow or deep copy of :mod:`codecs` streams
now raise a TypeError. Previously, stream pickling produced invalid data,
which attempts to read resulted in a RecursionError, as well as attempts to
create a copy of the stream.
Copy link
Member

Choose a reason for hiding this comment

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

Better wording: Previously, unpickled or copied streams could produce invalid data and attempts to read from them resulted in a RecursionError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, maybe my English is so bad that it was misunderstood by you. The problem is:

  1. Attempt to pickle a stream finished successfully. But the result is incorrect, and attempt to unpickle it fails with RecursionError.
  2. Attempt to copy a stream fails with RecursionError.

So there is no unpickled or copied stream. You cannot unpickle or copy.

How to say this in one sentence?

Copy link
Member

Choose a reason for hiding this comment

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

How about this:

Previously, both operations were possible, but produced wrong results and eventually failed with a RecursionError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, it is still not completely technically correct, because copying was not possible, and pickling caused a failure on other end. How about this:

Previously, pickling produced wrong results that eventually caused unpickling to fail with a RecursionError. Copying failed with a RecursionError.

or this:

Previously, copying failed with a RecursionError, while pickling produced wrong results that eventually caused unpickling to fail with a RecursionError.

Copy link
Member

Choose a reason for hiding this comment

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

Both are great. Sorry for the extra rounds 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. I always grateful for corrections.

@serhiy-storchaka serhiy-storchaka merged commit d6892c2 into python:main Sep 10, 2023
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the codecs-streams-no-pickle branch September 10, 2023 17:06
@bedevere-bot
Copy link

GH-109231 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Sep 10, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 10, 2023
@serhiy-storchaka @miss-islington
pythongh-50644: Forbid pickling of codecs streams (pythonGH-109180)
23232a7 
Attempts to pickle or create a shallow or deep copy of codecs streams
now raise a TypeError.

Previously, copying failed with a RecursionError, while pickling
produced wrong results that eventually caused unpickling to fail with
a RecursionError.
(cherry picked from commit d6892c2)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-bot
Copy link

GH-109232 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Sep 10, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 10, 2023
@serhiy-storchaka @miss-islington
pythongh-50644: Forbid pickling of codecs streams (pythonGH-109180)
89fe85a 
Attempts to pickle or create a shallow or deep copy of codecs streams
now raise a TypeError.

Previously, copying failed with a RecursionError, while pickling
produced wrong results that eventually caused unpickling to fail with
a RecursionError.
(cherry picked from commit d6892c2)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Sep 10, 2023
@miss-islington @serhiy-storchaka
[3.11] gh-50644: Forbid pickling of codecs streams (GH-109180) (GH-10…
b070d73 
…9232)

Attempts to pickle or create a shallow or deep copy of codecs streams
now raise a TypeError.

Previously, copying failed with a RecursionError, while pickling
produced wrong results that eventually caused unpickling to fail with
a RecursionError.
(cherry picked from commit d6892c2)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x Fedora LTO + PGO 3.11 has failed when building commit b070d73.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1023/builds/914) and take a look at the build logs.
  4. Check if the failure is related to this commit (b070d73) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1023/builds/914

Failed tests:

  • test_tools

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.11.edelsohn-fedora-z.lto-pgo/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_7f2a309e'


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.11.edelsohn-fedora-z.lto-pgo/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_7c29222d'


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.11.edelsohn-fedora-z.lto-pgo/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_07f4c78e'

Yhg1s pushed a commit that referenced this pull request Oct 2, 2023
@miss-islington @serhiy-storchaka
[3.12] gh-50644: Forbid pickling of codecs streams (GH-109180) (#109231)
3e1c9e8 
gh-50644: Forbid pickling of codecs streams (GH-109180)

Attempts to pickle or create a shallow or deep copy of codecs streams
now raise a TypeError.

Previously, copying failed with a RecursionError, while pickling
produced wrong results that eventually caused unpickling to fail with
a RecursionError.
(cherry picked from commit d6892c2)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malemburg malemburg malemburg approved these changes

Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serhiy-storchaka @miss-islington @bedevere-bot @malemburg