Deprecated Makefile in favor of StreamReader and StreamWriter

We replace the `MakeFile` factory function with direct class instantiation.
The `MakeFile` function now emits a `DeprecationWarning` and will be
removed in a future release.

The `pyOpenSSL` adapter still needs it own `makefile()` method
to provide wrapped streaming classes capable of handling `OpenSSL errors.

Author: julianz-

cheroot/connections.py

@@ -11,7 +11,6 @@
 
 from . import errors
 from ._compat import IS_WINDOWS
-from .makefile import MakeFile
 
 
 try:
@@ -304,7 +303,6 @@ def _from_server_socket(self, server_socket):  # noqa: C901  # FIXME
             if hasattr(s, 'settimeout'):
                 s.settimeout(self.server.timeout)
 
-            mf = MakeFile
             ssl_env = {}
             # if ssl cert and key are set, we try to be a secure HTTP server
             if self.server.ssl_adapter is not None:
@@ -327,12 +325,12 @@ def _from_server_socket(self, server_socket):  # noqa: C901  # FIXME
                     )
                     self._send_bad_request_plain_http_error(s)
                     return None
-                mf = self.server.ssl_adapter.makefile
+
                 # Re-apply our timeout since we may have a new socket object
                 if hasattr(s, 'settimeout'):
                     s.settimeout(self.server.timeout)
 
-            conn = self.server.ConnectionClass(self.server, s, mf)
+            conn = self.server.ConnectionClass(self.server, s)
 
             if not isinstance(self.server.bind_addr, (str, bytes)):
                 # optional values

cheroot/makefile.py

@@ -3,6 +3,7 @@
 # prefer slower Python-based io module
 import _pyio as io
 import socket
+from warnings import warn as _warn
 
 
 # Write only 16K at a time to sockets
@@ -70,6 +71,15 @@ def write(self, val, *args, **kwargs):
 
 
 def MakeFile(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE):
-    """File object attached to a socket object."""
+    """
+    File object attached to a socket object.
+
+    This method is now deprecated: Use StreamReader or StreamWriter directly.
+    """
+    _warn(
+        'MakeFile is deprecated. Use StreamReader or StreamWriter directly.',
+        DeprecationWarning,
+        stacklevel=2,
+    )
     cls = StreamReader if 'r' in mode else StreamWriter
     return cls(sock, mode, bufsize)

cheroot/server.py

@@ -84,7 +84,7 @@
 
 from . import __version__, connections, errors
 from ._compat import IS_PPC, bton
-from .makefile import MakeFile, StreamWriter
+from .makefile import StreamReader, StreamWriter
 from .workers import threadpool
 
 
@@ -1276,7 +1276,7 @@ class HTTPConnection:
     # Fields set by ConnectionManager.
     last_used = None
 
-    def __init__(self, server, sock, makefile=MakeFile):
+    def __init__(self, server, sock):
         """Initialize HTTPConnection instance.
 
         Args:
@@ -1287,8 +1287,18 @@ def __init__(self, server, sock, makefile=MakeFile):
         """
         self.server = server
         self.socket = sock
-        self.rfile = makefile(sock, 'rb', self.rbufsize)
-        self.wfile = makefile(sock, 'wb', self.wbufsize)
+
+        # Check if SSL adapter needs custom wrapped streaming
+        # objects (pyOpenSSL does, builtin doesn't)
+        if server.ssl_adapter is not None and hasattr(
+            server.ssl_adapter,
+            'makefile',
+        ):
+            self.rfile = server.ssl_adapter.makefile(sock, 'rb', self.rbufsize)
+            self.wfile = server.ssl_adapter.makefile(sock, 'wb', self.wbufsize)
+        else:
+            self.rfile = StreamReader(sock, 'rb', self.rbufsize)
+            self.wfile = StreamWriter(sock, 'wb', self.wbufsize)
         self.requests_seen = 0
 
         self.peercreds_enabled = self.server.peercreds_enabled

cheroot/server.pyi

@@ -112,7 +112,7 @@ class HTTPConnection:
     rfile: _t.Any
     wfile: _t.Any
     requests_seen: int
-    def __init__(self, server, sock, makefile=...) -> None: ...
+    def __init__(self, server, sock) -> None: ...
     def communicate(self): ...
     linger: bool
     def close(self) -> None: ...

cheroot/ssl/__init__.py

@@ -55,13 +55,16 @@ def _ensure_peer_speaks_https(raw_socket, /) -> None:
 
 
 class Adapter(ABC):
-    """Base class for SSL driver library adapters.
+    """
+    Base class for SSL driver library adapters.
 
     Required methods:
+        ``wrap(sock)``: Wrap a socket with SSL. Also returns ssl environ dict.
+        ``get_environ()``: Get SSL environment variables as a dict.
 
-        * ``wrap(sock) -> (wrapped socket, ssl environ dict)``
-        * ``makefile(sock, mode='r', bufsize=DEFAULT_BUFFER_SIZE) ->
-          socket file object``
+    Optional methods:
+        ``makefile(sock, mode, bufsize)``: Create custom file object.
+            The ``pyOpenSSL`` adapter uses this to handle SSL-specific errors.
     """
 
     @abstractmethod
@@ -108,8 +111,3 @@ def wrap(self, sock):
     def get_environ(self):
         """Return WSGI environ entries to be merged into each request."""
         raise NotImplementedError  # pragma: no cover
-
-    @abstractmethod
-    def makefile(self, sock, mode='r', bufsize=-1):
-        """Return socket file object."""
-        raise NotImplementedError  # pragma: no cover

cheroot/ssl/__init__.pyi

@@ -23,5 +23,3 @@ class Adapter(ABC):
     def wrap(self, sock): ...
     @abstractmethod
     def get_environ(self): ...
-    @abstractmethod
-    def makefile(self, sock, mode: str = ..., bufsize: int = ...): ...

cheroot/ssl/builtin.py

@@ -18,16 +18,7 @@
 except ImportError:
     ssl = None
 
-try:
-    from _pyio import DEFAULT_BUFFER_SIZE
-except ImportError:
-    try:
-        from io import DEFAULT_BUFFER_SIZE
-    except ImportError:
-        DEFAULT_BUFFER_SIZE = -1
-
 from .. import errors
-from ..makefile import StreamReader, StreamWriter
 from ..server import HTTPServer
 from . import Adapter
 
@@ -492,8 +483,3 @@ def _make_env_dn_dict(self, env_prefix, cert_value):
             for i, val in enumerate(values):
                 env['%s_%s_%i' % (env_prefix, attr_code, i)] = val
         return env
-
-    def makefile(self, sock, mode='r', bufsize=DEFAULT_BUFFER_SIZE):
-        """Return socket file object."""
-        cls = StreamReader if 'r' in mode else StreamWriter
-        return cls(sock, mode, bufsize)

cheroot/ssl/builtin.pyi

@@ -2,8 +2,6 @@ import typing as _t
 
 from . import Adapter
 
-DEFAULT_BUFFER_SIZE: int
-
 class BuiltinSSLAdapter(Adapter):
     CERT_KEY_TO_ENV: _t.Any
     CERT_KEY_TO_LDAP_CODE: _t.Any
@@ -22,4 +20,3 @@ class BuiltinSSLAdapter(Adapter):
     def context(self, context) -> None: ...
     def wrap(self, sock): ...
     def get_environ(self, sock): ...
-    def makefile(self, sock, mode: str = ..., bufsize: int = ...): ...

cheroot/test/test_makefile.py

@@ -1,5 +1,7 @@
 """Tests for :py:mod:`cheroot.makefile`."""
 
+import pytest
+
 from cheroot import makefile
 
 
@@ -39,7 +41,7 @@ def test_bytes_read():
     """Reader should capture bytes read."""
     sock = MockSocket()
     sock.messages.append(b'foo')
-    rfile = makefile.MakeFile(sock, 'r')
+    rfile = makefile.StreamReader(sock, 'r')
     rfile.read()
     assert rfile.bytes_read == 3
 
@@ -48,6 +50,21 @@ def test_bytes_written():
     """Writer should capture bytes written."""
     sock = MockSocket()
     sock.messages.append(b'foo')
-    wfile = makefile.MakeFile(sock, 'w')
+    wfile = makefile.StreamWriter(sock, 'w')
     wfile.write(b'bar')
     assert wfile.bytes_written == 3
+
+
+def test_makefile_deprecated():
+    """MakeFile function should emit a deprecation warning."""
+    sock = MockSocket()
+    sock.messages.append(b'foo')
+
+    with pytest.warns(
+        DeprecationWarning,
+        match='Use StreamReader or StreamWriter directly',
+    ) as warning_info:
+        makefile.MakeFile(sock, 'r')
+
+    assert len(warning_info) == 1
+    assert 'MakeFile is deprecated' in str(warning_info[0].message)

docs/changelog-fragments.d/805.deprecation.rst

@@ -0,0 +1,13 @@
+Deprecated :py:meth:`~cheroot.makefile.MakeFile` in favor of :py:class:`~cheroot.makefile.StreamReader`
+and :py:class:`~cheroot.makefile.StreamWriter`.
+
+The ``MakeFile`` factory function is replaced with direct class instantiation.
+The ``MakeFile`` function now emits a ``DeprecationWarning`` and will be
+removed in a future release.
+
+Adapter specific ``makefile()`` methods are removed from both :py:mod:`~cheroot.ssl.builtin.BuiltinSSLAdapter`
+and the :py:mod:`~cheroot.ssl.Adapter` interface but retained in :py:mod:`~cheroot.ssl.pyopenssl.pyOpenSSLAdapter,`
+which continues to need its own :py:meth:`~cheroot.ssl.pyopenssl.pyOpenSSLAdapter.makefile()`
+method to provide wrapped streaming classes equipped for dealing with ``OpenSSL`` errors.
+
+-- by :user:`julianz-`