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>

diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst
index fff7a7a03eb81dde4bdf0d803958880a90a9f0d7..a59817c1039210913ebf5848ddc2cb6447e1d9dc 100644 (file)
--- 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 45709cb06cf4e67659b0f2b7266615c20c32989a..62875540f8b92041446e7213d6debdf8fbee0fc6 100644 (file)
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -633,10 +633,9 @@ class SpooledTemporaryFile:
         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 @@ class SpooledTemporaryFile:
         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 f995f6c9bfaf0062a5b8187a603a20277cf0dd5f..232c5dae10fdf494be5e84f41cc110e70f82b0ed 100644 (file)
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -1114,7 +1114,8 @@ class TestSpooledTemporaryFile(BaseTestCase):
     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 @@ class TestSpooledTemporaryFile(BaseTestCase):
         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 @@ class TestSpooledTemporaryFile(BaseTestCase):
         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 @@ class TestSpooledTemporaryFile(BaseTestCase):
         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 (file)
index 0000000..a92b90a
--- /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.