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.

`makefile()` methods are also deprecated on the adapters and its
base class.

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,16 @@ 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 function 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,19 +1276,31 @@ class HTTPConnection:
     # Fields set by ConnectionManager.
     last_used = None
 
-    def __init__(self, server, sock, makefile=MakeFile):
+    def __init__(self, server, sock, makefile=None):
         """Initialize HTTPConnection instance.
 
         Args:
             server (HTTPServer): web server object receiving this request
             sock (socket._socketobject): the raw socket object (usually
                 TCP) for this connection
-            makefile (file): a fileobject class for reading from the socket
+            makefile (file): Now deprecated.
+                Used to be a fileobject class for reading from the socket.
         """
         self.server = server
         self.socket = sock
-        self.rfile = makefile(sock, 'rb', self.rbufsize)
-        self.wfile = makefile(sock, 'wb', self.wbufsize)
+
+        if makefile is not None:
+            _warn(
+                'The `makefile` parameter in creating an `HTTPConnection` '
+                'is deprecated and will be removed in a future version. '
+                'The connection socket should now be fully wrapped by the '
+                'adapter before being passed to this constructor.',
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        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
@@ -1358,12 +1370,8 @@ def communicate(self):  # noqa: C901  # FIXME
     def _handle_no_ssl(self, req):
         if not req or req.sent_headers:
             return
-        # Unwrap wfile
-        try:
-            resp_sock = self.socket._sock
-        except AttributeError:
-            # self.socket is of OpenSSL.SSL.Connection type
-            resp_sock = self.socket._socket
+        # Unwrap to get raw TCP socket
+        resp_sock = self.socket._sock
         self.wfile = StreamWriter(resp_sock, 'wb', self.wbufsize)
         msg = (
             'The client sent a plain HTTP request, but '

cheroot/ssl/__init__.py

@@ -56,13 +56,27 @@ 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) -> (wrapped socket, ssl environ dict)``
-        * ``makefile(sock, mode='r', bufsize=DEFAULT_BUFFER_SIZE) ->
-          socket file object``
+        * ``get_environ() -> (ssl environ dict)``
+
+    Optional methods:
+
+        * ``makefile(sock, mode='r', bufsize=-1) -> socket file object``
+
+        This method is deprecated and will be removed in a future release.
+
+        Historically, the ``PyOpenSSL`` adapter used ``makefile()`` to
+        wrap the underlying socket in an ``OpenSSL``-aware file object
+        so that Cheroot's HTTP request parser (which expects file-like I/O
+        such as ``readline()``) could read from TLS connections. The
+        adapter now fully wraps the socket in a TLSSocket object that
+        implements the necessary socket and file-like
+        methods directly, so ``makefile()`` is no longer needed.
     """
 
     @abstractmethod
@@ -110,9 +124,14 @@ 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."""
+        """
+        Return socket file object.
+
+        This method is now deprecated. It will be removed in a future version.
+
+        :raises NotImplementedError: Must be overridden by subclasses.
+        """
         raise NotImplementedError  # pragma: no cover
 
     def _prompt_for_tls_password(self) -> str:

cheroot/ssl/builtin.py

@@ -11,27 +11,39 @@
 import sys
 import threading
 from contextlib import suppress
+from warnings import warn as _warn
 
 
 try:
     import ssl
 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
 
 
+# WPS413 is to suppress linter error:
+# bad magic module function: __getattr__
+# DEFAULT_BUFFER_SIZE is deprecated so this module method will be removed
+# in a future release
+def __getattr__(name):  # noqa: WPS413
+    if name == 'DEFAULT_BUFFER_SIZE':
+        _warn(
+            (
+                '`DEFAULT_BUFFER_SIZE` is deprecated and '
+                'will be removed in a future release.'
+            ),
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        from io import DEFAULT_BUFFER_SIZE
+
+        return DEFAULT_BUFFER_SIZE
+    raise AttributeError(f'module {__name__!r} has no attribute {name!r}')
+
+
 def _assert_ssl_exc_contains(exc, *msgs):
     """Check whether SSL exception contains either of messages provided."""
     if len(msgs) < 1:
@@ -247,9 +259,6 @@ def __init__(
             private_key_password=private_key_password,
         )
 
-        if private_key_password is None:
-            private_key_password = self._prompt_for_tls_password
-
         self.context = ssl.create_default_context(
             purpose=ssl.Purpose.CLIENT_AUTH,
             cafile=certificate_chain,
@@ -496,7 +505,19 @@ def _make_env_dn_dict(self, env_prefix, cert_value):
                 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)
+    def makefile(self, sock, mode='r', bufsize=-1):
+        """
+        Return socket file object.
+
+        ``makefile`` is now deprecated and will be removed in a future
+        version.
+        """
+        _warn(
+            'The `makefile` method is deprecated and will be removed in a future version. '
+            'The connection socket should be fully wrapped by the adapter '
+            'before being passed to the HTTPConnection constructor.',
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
+        return sock.makefile(mode, bufsize)

cheroot/ssl/builtin.pyi

@@ -3,8 +3,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