https://github.com/python/cpython/commit/d21b8e82dda3caf0989bb39bc0e0ce2bfef... commit: d21b8e82dda3caf0989bb39bc0e0ce2bfef97081 branch: 3.8 author: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> committer: GitHub <noreply@github.com> date: 2019-11-27T21:23:14-08:00 summary: bpo-26730: Fix SpooledTemporaryFile data corruption (GH-17400) SpooledTemporaryFile.rollback() might cause data corruption when it is in text mode. Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit ea9835c5d154ab6a54eed627958473b6768b28cc) Co-authored-by: Inada Naoki <songofacandy@gmail.com> files: A Misc/NEWS.d/next/Library/2019-11-27-16-30-02.bpo-26730.56cdBn.rst M Doc/library/tempfile.rst M Lib/tempfile.py M Lib/test/test_tempfile.py diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst index fff7a7a03eb81..a59817c103921 100644 --- a/Doc/library/tempfile.rst +++ b/Doc/library/tempfile.rst @@ -105,8 +105,8 @@ The module defines the following user-callable items: causes the file to roll over to an on-disk file regardless of its size. The returned object is a file-like object whose :attr:`_file` attribute - is either an :class:`io.BytesIO` or :class:`io.StringIO` object (depending on - whether binary or text *mode* was specified) or a true file + is either an :class:`io.BytesIO` or :class:`io.TextIOWrapper` object + (depending on whether binary or text *mode* was specified) or a true file object, depending on whether :func:`rollover` has been called. This file-like object can be used in a :keyword:`with` statement, just like a normal file. diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 45709cb06cf4e..62875540f8b92 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -633,10 +633,9 @@ def __init__(self, max_size=0, mode='w+b', buffering=-1, if 'b' in mode: self._file = _io.BytesIO() else: - # Setting newline="\n" avoids newline translation; - # this is important because otherwise on Windows we'd - # get double newline translation upon rollover(). - self._file = _io.StringIO(newline="\n") + self._file = _io.TextIOWrapper(_io.BytesIO(), + encoding=encoding, errors=errors, + newline=newline) self._max_size = max_size self._rolled = False self._TemporaryFileArgs = {'mode': mode, 'buffering': buffering, @@ -656,8 +655,12 @@ def rollover(self): newfile = self._file = TemporaryFile(**self._TemporaryFileArgs) del self._TemporaryFileArgs - newfile.write(file.getvalue()) - newfile.seek(file.tell(), 0) + pos = file.tell() + if hasattr(newfile, 'buffer'): + newfile.buffer.write(file.detach().getvalue()) + else: + newfile.write(file.getvalue()) + newfile.seek(pos, 0) self._rolled = True diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index f995f6c9bfaf0..232c5dae10fdf 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1114,7 +1114,8 @@ def test_properties(self): def test_text_mode(self): # Creating a SpooledTemporaryFile with a text mode should produce # a file object reading and writing (Unicode) text strings. - f = tempfile.SpooledTemporaryFile(mode='w+', max_size=10) + f = tempfile.SpooledTemporaryFile(mode='w+', max_size=10, + encoding="utf-8") f.write("abc\n") f.seek(0) self.assertEqual(f.read(), "abc\n") @@ -1124,9 +1125,9 @@ def test_text_mode(self): self.assertFalse(f._rolled) self.assertEqual(f.mode, 'w+') self.assertIsNone(f.name) - self.assertIsNone(f.newlines) - self.assertIsNone(f.encoding) - self.assertIsNone(f.errors) + self.assertEqual(f.newlines, os.linesep) + self.assertEqual(f.encoding, "utf-8") + self.assertEqual(f.errors, "strict") f.write("xyzzy\n") f.seek(0) @@ -1139,8 +1140,8 @@ def test_text_mode(self): self.assertEqual(f.mode, 'w+') self.assertIsNotNone(f.name) self.assertEqual(f.newlines, os.linesep) - self.assertIsNotNone(f.encoding) - self.assertIsNotNone(f.errors) + self.assertEqual(f.encoding, "utf-8") + self.assertEqual(f.errors, "strict") def test_text_newline_and_encoding(self): f = tempfile.SpooledTemporaryFile(mode='w+', max_size=10, @@ -1152,13 +1153,15 @@ def test_text_newline_and_encoding(self): self.assertFalse(f._rolled) self.assertEqual(f.mode, 'w+') self.assertIsNone(f.name) - self.assertIsNone(f.newlines) - self.assertIsNone(f.encoding) - self.assertIsNone(f.errors) + self.assertIsNotNone(f.newlines) + self.assertEqual(f.encoding, "utf-8") + self.assertEqual(f.errors, "ignore") - f.write("\u039B" * 20 + "\r\n") + f.write("\u039C" * 10 + "\r\n") + f.write("\u039D" * 20) f.seek(0) - self.assertEqual(f.read(), "\u039B\r\n" + ("\u039B" * 20) + "\r\n") + self.assertEqual(f.read(), + "\u039B\r\n" + ("\u039C" * 10) + "\r\n" + ("\u039D" * 20)) self.assertTrue(f._rolled) self.assertEqual(f.mode, 'w+') self.assertIsNotNone(f.name) diff --git a/Misc/NEWS.d/next/Library/2019-11-27-16-30-02.bpo-26730.56cdBn.rst b/Misc/NEWS.d/next/Library/2019-11-27-16-30-02.bpo-26730.56cdBn.rst new file mode 100644 index 0000000000000..a92b90a495605 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-11-27-16-30-02.bpo-26730.56cdBn.rst @@ -0,0 +1,2 @@ +Fix ``SpooledTemporaryFile.rollover()`` might corrupt the file when it is in +text mode. Patch by Serhiy Storchaka.