Merge pull request #1546 from gevent/issue1542

Fix AttributeError when wrapping a FileObject around already-opened text streams

CHANGES.rst

@@ -8,6 +8,9 @@
 ==================
 
 - Make `gevent.lock.RLock.acquire` accept the *timeout* parameter.
+- Fix an ``AttributeError`` when wrapping gevent's ``FileObject``
+  around an opened text stream. Reported in :issue:`1542` by
+  dmrlawson.
 
 
 1.5a4 (2020-03-23)

src/gevent/_fileobjectcommon.py

@@ -222,8 +222,8 @@ class OpenDescriptor(object): # pylint:disable=too-many-instance-attributes
 
     def wrapped(self, raw):
         """
-        Wraps the raw IO object (`RawIOBase`) in buffers, text decoding,
-        and newline handling.
+        Wraps the raw IO object (`RawIOBase` or `io.TextIOBase`) in
+        buffers, text decoding, and newline handling.
         """
         # pylint:disable=too-many-branches
         result = raw
@@ -245,40 +245,59 @@ class OpenDescriptor(object): # pylint:disable=too-many-instance-attributes
         if buffering < 0: # pragma: no cover
             raise ValueError("invalid buffering size")
 
-        if buffering != 0:
-            if self.updating:
-                Buffer = io.BufferedRandom
-            elif self.creating or self.writing or self.appending:
-                Buffer = io.BufferedWriter
-            elif self.reading:
-                Buffer = io.BufferedReader
-            else: # prgama: no cover
-                raise ValueError("unknown mode: %r" % self.mode)
-
-            try:
-                result = Buffer(raw, buffering)
-            except AttributeError:
-                # Python 2 file() objects don't have the readable/writable
-                # attributes. But they handle their own buffering.
-                result = raw
+        if not isinstance(raw, io.BufferedIOBase) and \
+           (not hasattr(raw, 'buffer') or raw.buffer is None):
+            # Need to wrap our own buffering around it. If it
+            # is already buffered, don't do so.
+            if buffering != 0:
+                if self.updating:
+                    Buffer = io.BufferedRandom
+                elif self.creating or self.writing or self.appending:
+                    Buffer = io.BufferedWriter
+                elif self.reading:
+                    Buffer = io.BufferedReader
+                else: # prgama: no cover
+                    raise ValueError("unknown mode: %r" % self.mode)
+
+                try:
+                    result = Buffer(raw, buffering)
+                except AttributeError:
+                    # Python 2 file() objects don't have the readable/writable
+                    # attributes. But they handle their own buffering.
+                    result = raw
 
         if self.binary:
+            if isinstance(raw, io.TextIOBase):
+                # Can't do it. The TextIO object will have its own buffer, and
+                # trying to read from the raw stream or the buffer without going through
+                # the TextIO object is likely to lead to problems with the codec.
+                raise ValueError("Unable to perform binary IO on top of text IO stream")
             return result
+
+        # Either native or text at this point.
         if PY2 and self.native:
             # Neither text mode nor binary mode specified.
             if self.universal:
                 # universal was requested, e.g., 'rU'
                 result = UniversalNewlineBytesWrapper(result, line_buffering)
         else:
-            result = io.TextIOWrapper(result, self.encoding, self.errors, self.newline,
-                                      line_buffering)
+            # Python 2 and text mode, or Python 3 and either text or native (both are the same)
+            if not isinstance(raw, io.TextIOBase):
+                # Avoid double-wrapping a TextIOBase in another TextIOWrapper.
+                # That tends not to work. See https://github.com/gevent/gevent/issues/1542
+                result = io.TextIOWrapper(result, self.encoding, self.errors, self.newline,
+                                          line_buffering)
+
+        if result is not raw:
+            # Set the mode, if possible, but only if we created a new
+            # object.
+            try:
+                result.mode = self.mode
+            except (AttributeError, TypeError):
+                # AttributeError: No such attribute
+                # TypeError: Readonly attribute (py2)
+                pass
 
-        try:
-            result.mode = self.mode
-        except (AttributeError, TypeError):
-            # AttributeError: No such attribute
-            # TypeError: Readonly attribute (py2)
-            pass
         return result
 
 

src/gevent/lock.py

@@ -188,8 +188,9 @@ class DummySemaphore(object):
     def unlink(self, callback):
         pass
 
-    def wait(self, timeout=None):
+    def wait(self, timeout=None): # pylint:disable=unused-argument
         """Waiting for a DummySemaphore returns immediately."""
+        return 1
 
     def acquire(self, blocking=True, timeout=None):
         """

src/gevent/tests/test__fileobject.py

-from __future__ import print_function
+from __future__ import print_function, absolute_import
 
 import functools
 import gc
@@ -133,7 +133,8 @@ class TestFileObjectBlock(greentest.TestCase): # serves as a base for the concur
         self.assertEqual(native_data, fileobj_data)
 
     def __check_native_matches(self, byte_data, open_mode,
-                               meth='read', **open_kwargs):
+                               meth='read', open_path=True,
+                               **open_kwargs):
         fileno, path = tempfile.mkstemp('.gevent_test_' + open_mode)
         self.addCleanup(os.remove, path)
 
@@ -143,8 +144,16 @@ class TestFileObjectBlock(greentest.TestCase): # serves as a base for the concur
         with io.open(path, open_mode, **open_kwargs) as f:
             native_data = getattr(f, meth)()
 
-        with self._makeOne(path, open_mode, **open_kwargs) as f:
-            gevent_data = getattr(f, meth)()
+        if open_path:
+            with self._makeOne(path, open_mode, **open_kwargs) as f:
+                gevent_data = getattr(f, meth)()
+        else:
+            # Note that we don't use ``io.open()`` for the raw file,
+            # on Python 2. We want 'r' to mean what the usual call to open() means.
+            opener = io.open if PY3 else open
+            with opener(path, open_mode, **open_kwargs) as raw:
+                with self._makeOne(raw) as f:
+                    gevent_data = getattr(f, meth)()
 
         self.assertEqual(native_data, gevent_data)
         return gevent_data
@@ -171,7 +180,7 @@ class TestFileObjectBlock(greentest.TestCase): # serves as a base for the concur
         pass
 
     @skipUnlessWorksWithRegularFiles
-    def test_rbU_produces_bytes(self):
+    def test_rbU_produces_bytes_readline(self):
         # Including U in rb still produces bytes.
         # Note that the universal newline behaviour is
         # essentially ignored in explicit bytes mode.
@@ -192,6 +201,24 @@ class TestFileObjectBlock(greentest.TestCase): # serves as a base for the concur
         )
         self.assertIsInstance(gevent_data[0], str)
 
+    @skipUnlessWorksWithRegularFiles
+    def test_r_readline_produces_native(self):
+        gevent_data = self.__check_native_matches(
+            b'line1\n',
+            'r',
+            meth='readline',
+        )
+        self.assertIsInstance(gevent_data, str)
+
+    @skipUnlessWorksWithRegularFiles
+    def test_r_readline_on_fobject_produces_native(self):
+        gevent_data = self.__check_native_matches(
+            b'line1\n',
+            'r',
+            meth='readline',
+            open_path=False,
+        )
+        self.assertIsInstance(gevent_data, str)
 
     def test_close_pipe(self):
         # Issue #190, 203

tox.ini

@@ -3,6 +3,7 @@ envlist =
     py27,py35,py36,py37,py27-cffi,pypy,pypy3,py27-libuv,lint
 
 [testenv]
+usedevelop = true
 extras =
     test
     events