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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Lib/codecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,9 @@ def __enter__(self):
def __exit__(self, type, value, tb):
self.stream.close()

def __reduce_ex__(self, proto):
raise TypeError("can't serialize %s" % self.__class__.__name__)

###

class StreamReader(Codec):
Expand Down Expand Up @@ -663,6 +666,9 @@ def __enter__(self):
def __exit__(self, type, value, tb):
self.stream.close()

def __reduce_ex__(self, proto):
raise TypeError("can't serialize %s" % self.__class__.__name__)

###

class StreamReaderWriter:
Expand Down Expand Up @@ -750,6 +756,9 @@ def __enter__(self):
def __exit__(self, type, value, tb):
self.stream.close()

def __reduce_ex__(self, proto):
raise TypeError("can't serialize %s" % self.__class__.__name__)

###

class StreamRecoder:
Expand Down Expand Up @@ -866,6 +875,9 @@ def __enter__(self):
def __exit__(self, type, value, tb):
self.stream.close()

def __reduce_ex__(self, proto):
raise TypeError("can't serialize %s" % self.__class__.__name__)

### Shortcuts

def open(filename, mode='r', encoding=None, errors='strict', buffering=-1):
Expand Down
79 changes: 79 additions & 0 deletions Lib/test/test_codecs.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import codecs
import contextlib
import copy
import io
import locale
import pickle
import sys
import unittest
import encodings
Expand Down Expand Up @@ -1771,6 +1773,61 @@ def test_readlines(self):
f = self.reader(self.stream)
self.assertEqual(f.readlines(), ['\ud55c\n', '\uae00'])

def test_copy(self):
f = self.reader(Queue(b'\xed\x95\x9c\n\xea\xb8\x80'))
with self.assertRaisesRegex(TypeError, 'StreamReader'):
copy.copy(f)
with self.assertRaisesRegex(TypeError, 'StreamReader'):
copy.deepcopy(f)

def test_pickle(self):
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
with self.subTest(protocol=proto):
f = self.reader(Queue(b'\xed\x95\x9c\n\xea\xb8\x80'))
with self.assertRaisesRegex(TypeError, 'StreamReader'):
pickle.dumps(f, proto)


class StreamWriterTest(unittest.TestCase):

def setUp(self):
self.writer = codecs.getwriter('utf-8')

def test_copy(self):
f = self.writer(Queue(b''))
with self.assertRaisesRegex(TypeError, 'StreamWriter'):
copy.copy(f)
with self.assertRaisesRegex(TypeError, 'StreamWriter'):
copy.deepcopy(f)

def test_pickle(self):
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
with self.subTest(protocol=proto):
f = self.writer(Queue(b''))
with self.assertRaisesRegex(TypeError, 'StreamWriter'):
pickle.dumps(f, proto)


class StreamReaderWriterTest(unittest.TestCase):

def setUp(self):
self.reader = codecs.getreader('latin1')
self.writer = codecs.getwriter('utf-8')

def test_copy(self):
f = codecs.StreamReaderWriter(Queue(b''), self.reader, self.writer)
with self.assertRaisesRegex(TypeError, 'StreamReaderWriter'):
copy.copy(f)
with self.assertRaisesRegex(TypeError, 'StreamReaderWriter'):
copy.deepcopy(f)

def test_pickle(self):
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
with self.subTest(protocol=proto):
f = codecs.StreamReaderWriter(Queue(b''), self.reader, self.writer)
with self.assertRaisesRegex(TypeError, 'StreamReaderWriter'):
pickle.dumps(f, proto)


class EncodedFileTest(unittest.TestCase):

Expand Down Expand Up @@ -3346,6 +3403,28 @@ def test_seeking_write(self):
self.assertEqual(sr.readline(), b'abc\n')
self.assertEqual(sr.readline(), b'789\n')

def test_copy(self):
bio = io.BytesIO()
codec = codecs.lookup('ascii')
sr = codecs.StreamRecoder(bio, codec.encode, codec.decode,
encodings.ascii.StreamReader, encodings.ascii.StreamWriter)

with self.assertRaisesRegex(TypeError, 'StreamRecoder'):
copy.copy(sr)
with self.assertRaisesRegex(TypeError, 'StreamRecoder'):
copy.deepcopy(sr)

def test_pickle(self):
q = Queue(b'')
codec = codecs.lookup('ascii')
sr = codecs.StreamRecoder(q, codec.encode, codec.decode,
encodings.ascii.StreamReader, encodings.ascii.StreamWriter)

for proto in range(pickle.HIGHEST_PROTOCOL + 1):
with self.subTest(protocol=proto):
with self.assertRaisesRegex(TypeError, 'StreamRecoder'):
pickle.dumps(sr, proto)


@unittest.skipIf(_testinternalcapi is None, 'need _testinternalcapi module')
class LocaleCodecTest(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
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.