X Tutup
Skip to content

gh-50644: Forbid pickling of codecs streams#109180

Merged
serhiy-storchaka merged 2 commits intopython:mainfrom
serhiy-storchaka:codecs-streams-no-pickle
Sep 10, 2023
Merged

gh-50644: Forbid pickling of codecs streams#109180
serhiy-storchaka merged 2 commits intopython:mainfrom
serhiy-storchaka:codecs-streams-no-pickle

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

@malemburg malemburg Sep 9, 2023

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

@serhiy-storchaka serhiy-storchaka Sep 9, 2023

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

@malemburg malemburg Sep 9, 2023

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

@serhiy-storchaka serhiy-storchaka Sep 10, 2023

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

@malemburg malemburg Sep 10, 2023

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

@serhiy-storchaka serhiy-storchaka Sep 10, 2023

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

miss-islington commented Sep 10, 2023

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

bedevere-bot commented Sep 10, 2023

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 <storchaka@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Sep 10, 2023

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 <storchaka@gmail.com>
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 <storchaka@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Sep 10, 2023

⚠️⚠️⚠️ 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 <storchaka@gmail.com>
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
X Tutup