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-

.flake8

@@ -137,6 +137,7 @@ per-file-ignores =
   cheroot/ssl/pyopenssl.py: C815, DAR101, DAR201, DAR401, I001, I003, I005, N801, N804, RST304, WPS100, WPS110, WPS111, WPS117, WPS120, WPS121, WPS130, WPS210, WPS220, WPS221, WPS225, WPS229, WPS231, WPS238, WPS301, WPS335, WPS338, WPS420, WPS422, WPS430, WPS432, WPS501, WPS504, WPS505, WPS601, WPS608, WPS615
   cheroot/test/conftest.py: DAR101, DAR201, DAR301, I001, I003, I005, WPS100, WPS130, WPS325, WPS354, WPS420, WPS422, WPS430, WPS457
   cheroot/test/helper.py: DAR101, DAR201, DAR401, I001, I003, I004, N802, WPS110, WPS111, WPS121, WPS201, WPS220, WPS231, WPS301, WPS414, WPS421, WPS422, WPS505
+  cheroot/test/ssl/test_ssl_pyopenssl.py: DAR101, DAR201, WPS202, WPS118, WPS204, WPS226, WPS441
   cheroot/test/test_cli.py: DAR101, DAR201, I001, I005, N802, S101, S108, WPS110, WPS421, WPS431, WPS473
   cheroot/test/test_makefile.py: DAR101, DAR201, I004, RST304, S101, WPS110, WPS122
   cheroot/test/test_wsgi.py: DAR101, DAR301, I001, I004, S101, WPS110, WPS111, WPS117, WPS118, WPS121, WPS210, WPS421, WPS430, WPS432, WPS441, WPS509

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,10 +84,16 @@
 
 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
 
 
+try:
+    from OpenSSL import SSL
+except ImportError:
+    SSL = None  # OpenSSL not available
+
+
 __all__ = (
     'ChunkedRFile',
     'DropUnderscoreHeaderReader',
@@ -1276,19 +1282,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 +1376,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 '
@@ -1507,20 +1521,23 @@ def peer_group(self):
 
     def _close_kernel_socket(self):
         """Terminate the connection at the transport level."""
-        # Honor ``sock_shutdown`` for PyOpenSSL connections.
-        shutdown = getattr(
-            self.socket,
-            'sock_shutdown',
-            self.socket.shutdown,
-        )
-
         try:
-            shutdown(socket.SHUT_RDWR)  # actually send a TCP FIN
+            self.socket.shutdown(socket.SHUT_RDWR)
         except errors.acceptable_sock_shutdown_exceptions:
             pass
         except socket.error as e:
             if e.errno not in errors.acceptable_sock_shutdown_error_codes:
                 raise
+        except Exception as exc:
+            # translate SSL exceptions to FatalSSLAlert
+            if SSL is not None and isinstance(
+                exc,
+                (SSL.Error, SSL.SysCallError),
+            ):
+                raise errors.FatalSSLAlert(
+                    'TLS/SSL connection failure',
+                ) from exc
+            raise  # re-raise everything else
 
 
 class HTTPServer:

cheroot/ssl/__init__.py

@@ -61,8 +61,21 @@ class Adapter(ABC):
     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
+        provides the necessary socket and file-like
+        methods directly, so ``makefile()`` is no longer needed.
     """
 
     @abstractmethod
@@ -110,9 +123,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:
@@ -496,7 +508,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