Skip to content

Commit 5b37f53

Browse files
committed
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.
1 parent c5d5175 commit 5b37f53

11 files changed

Lines changed: 329 additions & 72 deletions

File tree

cheroot/connections.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
from . import errors
1313
from ._compat import IS_WINDOWS
14-
from .makefile import MakeFile
1514

1615

1716
try:
@@ -304,7 +303,6 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
304303
if hasattr(s, 'settimeout'):
305304
s.settimeout(self.server.timeout)
306305

307-
mf = MakeFile
308306
ssl_env = {}
309307
# if ssl cert and key are set, we try to be a secure HTTP server
310308
if self.server.ssl_adapter is not None:
@@ -327,12 +325,12 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
327325
)
328326
self._send_bad_request_plain_http_error(s)
329327
return None
330-
mf = self.server.ssl_adapter.makefile
328+
331329
# Re-apply our timeout since we may have a new socket object
332330
if hasattr(s, 'settimeout'):
333331
s.settimeout(self.server.timeout)
334332

335-
conn = self.server.ConnectionClass(self.server, s, mf)
333+
conn = self.server.ConnectionClass(self.server, s)
336334

337335
if not isinstance(self.server.bind_addr, (str, bytes)):
338336
# optional values

cheroot/makefile.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# prefer slower Python-based io module
44
import _pyio as io
55
import socket
6+
from warnings import warn as _warn
67

78

89
# Write only 16K at a time to sockets
@@ -70,6 +71,16 @@ def write(self, val, *args, **kwargs):
7071

7172

7273
def MakeFile(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE):
73-
"""File object attached to a socket object."""
74+
"""
75+
File object attached to a socket object.
76+
77+
This function is now deprecated: Use
78+
StreamReader or StreamWriter directly.
79+
"""
80+
_warn(
81+
'MakeFile is deprecated. Use StreamReader or StreamWriter directly.',
82+
DeprecationWarning,
83+
stacklevel=2,
84+
)
7485
cls = StreamReader if 'r' in mode else StreamWriter
7586
return cls(sock, mode, bufsize)

cheroot/server.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484

8585
from . import __version__, connections, errors
8686
from ._compat import IS_PPC, bton
87-
from .makefile import MakeFile, StreamWriter
87+
from .makefile import StreamReader, StreamWriter
8888
from .workers import threadpool
8989

9090

@@ -1276,19 +1276,31 @@ class HTTPConnection:
12761276
# Fields set by ConnectionManager.
12771277
last_used = None
12781278

1279-
def __init__(self, server, sock, makefile=MakeFile):
1279+
def __init__(self, server, sock, makefile=None):
12801280
"""Initialize HTTPConnection instance.
12811281
12821282
Args:
12831283
server (HTTPServer): web server object receiving this request
12841284
sock (socket._socketobject): the raw socket object (usually
12851285
TCP) for this connection
1286-
makefile (file): a fileobject class for reading from the socket
1286+
makefile (file): Now deprecated
1287+
Use to be a fileobject class for reading from the socket
12871288
"""
12881289
self.server = server
12891290
self.socket = sock
1290-
self.rfile = makefile(sock, 'rb', self.rbufsize)
1291-
self.wfile = makefile(sock, 'wb', self.wbufsize)
1291+
1292+
if makefile is not None:
1293+
_warn(
1294+
'The `makefile` parameter in creating an `HTTPConnection` '
1295+
'is deprecated and will be removed in a future version. '
1296+
'The connection socket should now be fully wrapped by the '
1297+
'adapter before being passed to this constructor.',
1298+
DeprecationWarning,
1299+
stacklevel=2,
1300+
)
1301+
1302+
self.rfile = StreamReader(sock, 'rb', self.rbufsize)
1303+
self.wfile = StreamWriter(sock, 'wb', self.wbufsize)
12921304
self.requests_seen = 0
12931305

12941306
self.peercreds_enabled = self.server.peercreds_enabled
@@ -1358,12 +1370,8 @@ def communicate(self): # noqa: C901 # FIXME
13581370
def _handle_no_ssl(self, req):
13591371
if not req or req.sent_headers:
13601372
return
1361-
# Unwrap wfile
1362-
try:
1363-
resp_sock = self.socket._sock
1364-
except AttributeError:
1365-
# self.socket is of OpenSSL.SSL.Connection type
1366-
resp_sock = self.socket._socket
1373+
# Unwrap to get raw TCP socket
1374+
resp_sock = self.socket._sock
13671375
self.wfile = StreamWriter(resp_sock, 'wb', self.wbufsize)
13681376
msg = (
13691377
'The client sent a plain HTTP request, but '

cheroot/ssl/__init__.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,24 @@ def _ensure_peer_speaks_https(raw_socket, /) -> None:
5656

5757

5858
class Adapter(ABC):
59-
"""Base class for SSL driver library adapters.
59+
"""
60+
Base class for SSL driver library adapters.
6061
6162
Required methods:
62-
63-
* ``wrap(sock) -> (wrapped socket, ssl environ dict)``
64-
* ``makefile(sock, mode='r', bufsize=DEFAULT_BUFFER_SIZE) ->
65-
socket file object``
63+
``wrap(sock)``: Wrap a socket with SSL. Also returns ssl environ dict.
64+
``get_environ()``: Get SSL environment variables as a dict.
65+
66+
Optional methods:
67+
``makefile(sock, mode, bufsize)``: Create custom file object.
68+
This method is deprecated and will be removed in a future release.
69+
70+
Historically, the ``PyOpenSSL`` adapter used ``makefile()`` to
71+
wrap the underlying socket in an ``OpenSSL``-aware file object
72+
so that Cheroot's HTTP request parser (which expects file-like I/O
73+
such as ``readline()``) could read from TLS connections. The
74+
adapter now fully wraps the socket in a TLSSocket object that
75+
implements the necessary socket and file-like
76+
methods directly, so makefile() is no longer needed.
6677
"""
6778

6879
@abstractmethod
@@ -112,7 +123,11 @@ def get_environ(self):
112123

113124
@abstractmethod
114125
def makefile(self, sock, mode='r', bufsize=-1):
115-
"""Return socket file object."""
126+
"""
127+
Return socket file object.
128+
129+
This method is now deprecated. It will be removed in a future version.
130+
"""
116131
raise NotImplementedError # pragma: no cover
117132

118133
def _prompt_for_tls_password(self) -> str:

cheroot/ssl/builtin.py

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,42 @@
77
``BuiltinSSLAdapter``.
88
"""
99

10+
import io
1011
import socket
1112
import sys
1213
import threading
1314
from contextlib import suppress
15+
from warnings import warn as _warn
1416

1517

1618
try:
1719
import ssl
1820
except ImportError:
1921
ssl = None
2022

21-
try:
22-
from _pyio import DEFAULT_BUFFER_SIZE
23-
except ImportError:
24-
try:
25-
from io import DEFAULT_BUFFER_SIZE
26-
except ImportError:
27-
DEFAULT_BUFFER_SIZE = -1
28-
2923
from .. import errors
30-
from ..makefile import StreamReader, StreamWriter
3124
from ..server import HTTPServer
3225
from . import Adapter
3326

3427

28+
# WPS413 is to suppress linter error:
29+
# bad magic module function: __getattr__
30+
# DEFAULT_BUFFER_SIZE is deprecated so this module method will be removed
31+
# in a future release
32+
def __getattr__(name): # noqa: WPS413
33+
if name == 'DEFAULT_BUFFER_SIZE':
34+
_warn(
35+
(
36+
'`DEFAULT_BUFFER_SIZE` is deprecated and '
37+
'will be removed in a future release.'
38+
),
39+
DeprecationWarning,
40+
stacklevel=2,
41+
)
42+
return io.DEFAULT_BUFFER_SIZE
43+
raise AttributeError(f'module {__name__!r} has no attribute {name!r}')
44+
45+
3546
def _assert_ssl_exc_contains(exc, *msgs):
3647
"""Check whether SSL exception contains either of messages provided."""
3748
if len(msgs) < 1:
@@ -496,7 +507,19 @@ def _make_env_dn_dict(self, env_prefix, cert_value):
496507
env['%s_%s_%i' % (env_prefix, attr_code, i)] = val
497508
return env
498509

499-
def makefile(self, sock, mode='r', bufsize=DEFAULT_BUFFER_SIZE):
500-
"""Return socket file object."""
501-
cls = StreamReader if 'r' in mode else StreamWriter
502-
return cls(sock, mode, bufsize)
510+
def makefile(self, sock, mode='r', bufsize=-1):
511+
"""
512+
Return socket file object.
513+
514+
``makefile`` is now deprecated and will be removed in a future
515+
version.
516+
"""
517+
_warn(
518+
'The `makefile` method is deprecated and will be removed in a future version. '
519+
'The connection socket should be fully wrapped by the adapter '
520+
'before being passed to the HTTPConnection constructor.',
521+
DeprecationWarning,
522+
stacklevel=2,
523+
)
524+
525+
return sock.makefile(mode, bufsize)

cheroot/ssl/builtin.pyi

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import typing as _t
33

44
from . import Adapter
55

6-
DEFAULT_BUFFER_SIZE: int
7-
86
class BuiltinSSLAdapter(Adapter):
97
CERT_KEY_TO_ENV: _t.Any
108
CERT_KEY_TO_LDAP_CODE: _t.Any

cheroot/ssl/pyopenssl.py

Lines changed: 92 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import sys
5555
import threading
5656
import time
57+
from contextlib import suppress as _suppress
5758
from warnings import warn as _warn
5859

5960

@@ -74,7 +75,6 @@
7475
errors,
7576
server as cheroot_server,
7677
)
77-
from ..makefile import StreamReader, StreamWriter
7878
from . import Adapter
7979

8080

@@ -168,14 +168,6 @@ def send(self, *args, **kwargs):
168168
)
169169

170170

171-
class SSLFileobjectStreamReader(SSLFileobjectMixin, StreamReader):
172-
"""SSL file object attached to a socket object."""
173-
174-
175-
class SSLFileobjectStreamWriter(SSLFileobjectMixin, StreamWriter):
176-
"""SSL file object attached to a socket object."""
177-
178-
179171
class SSLConnectionProxyMeta:
180172
"""Metaclass for generating a bunch of proxy methods."""
181173

@@ -345,9 +337,11 @@ def wrap(self, sock):
345337
)
346338

347339
conn = SSLConnection(self.context, sock)
348-
349340
conn.set_accept_state() # Tell OpenSSL this is a server connection
350-
return conn, self._environ.copy()
341+
342+
# Wrap the SSLConnection to provide standard socket interface
343+
tls_socket = TLSSocket(conn)
344+
return tls_socket, self._environ.copy()
351345

352346
def _password_callback(
353347
self,
@@ -461,17 +455,91 @@ def get_environ(self):
461455
return ssl_environ
462456

463457
def makefile(self, sock, mode='r', bufsize=-1):
464-
"""Return socket file object."""
465-
cls = (
466-
SSLFileobjectStreamReader
467-
if 'r' in mode
468-
else SSLFileobjectStreamWriter
458+
"""
459+
Return socket file object.
460+
461+
``makefile`` is now deprecated and will be removed in a future
462+
version.
463+
"""
464+
_warn(
465+
'The `makefile` method is deprecated and will be removed in a future version. '
466+
'The connection socket should be fully wrapped by the adapter '
467+
'before being passed to the HTTPConnection constructor.',
468+
DeprecationWarning,
469+
stacklevel=2,
469470
)
470-
# sock is an pyopenSSL.SSLConnection instance here
471-
if SSL and isinstance(sock, SSLConnection):
472-
wrapped_socket = cls(sock, mode, bufsize)
473-
wrapped_socket.ssl_timeout = sock.gettimeout()
474-
return wrapped_socket
475-
# This is from past:
476-
# TODO: figure out what it's meant for
477-
return cheroot_server.CP_fileobject(sock, mode, bufsize)
471+
472+
return sock.makefile(mode, bufsize)
473+
474+
475+
class TLSSocket(SSLFileobjectMixin):
476+
"""
477+
Wrap ``pyOpenSSL`` SSL.Connection to work with StreamReader/StreamWriter.
478+
479+
Handles OpenSSL-specific errors internally so that standard Python I/O
480+
classes can use it transparently.
481+
"""
482+
483+
def __init__(self, ssl_conn, ssl_timeout=None, ssl_retry=0.01):
484+
"""Initialize with an SSL.Connection object."""
485+
self._ssl_conn = ssl_conn
486+
self._sock = ssl_conn._socket # Store reference to raw TCP socket
487+
self._lock = threading.RLock()
488+
self.ssl_timeout = ssl_timeout or 3.0
489+
self.ssl_retry = ssl_retry
490+
491+
# Socket I/O
492+
# _safe_call is delegated to the SSLFileobjectMixin
493+
494+
def recv_into(self, buffer, nbytes=None):
495+
"""Receive data into a buffer (socket interface)."""
496+
with self._lock:
497+
result = self._safe_call(
498+
True,
499+
self._ssl_conn.recv_into,
500+
buffer,
501+
nbytes,
502+
)
503+
# Handle the case where _safe_call returns b'' for EOF
504+
if result == b'':
505+
return 0
506+
return result
507+
508+
def send(self, data):
509+
"""Send data (socket interface)."""
510+
with self._lock:
511+
return self._safe_call(
512+
False, # is_reader=False
513+
self._ssl_conn.send,
514+
data,
515+
)
516+
517+
def fileno(self):
518+
"""Return the file descriptor."""
519+
return self._ssl_conn.fileno()
520+
521+
def _decref_socketios(self):
522+
"""Shutdown the connection."""
523+
return self._ssl_conn._decref_socketios()
524+
525+
def shutdown(self, how):
526+
"""Shutdown the connection."""
527+
# SSL shutdown can fail if the handshake didn't complete or during
528+
# error conditions. Since shutdown is a cleanup operation, we suppress
529+
# all errors - the connection will be closed anyway.
530+
with _suppress(SSL.Error, socket.error, OSError):
531+
return self._ssl_conn.shutdown(how)
532+
533+
def close(self):
534+
"""Close the TLS socket and underlying connection."""
535+
# 1. Attempt an SSL-level shutdown
536+
with _suppress(SSL.Error, OSError):
537+
self._ssl_conn.shutdown()
538+
539+
# 2. Close the SSL connection object
540+
with _suppress(SSL.Error, OSError):
541+
self._ssl_conn.close()
542+
543+
# 3. Close the raw TCP socket
544+
with _suppress(OSError):
545+
self._sock.close()

0 commit comments

Comments
 (0)