Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9aac18d
Refactor of _from_server_socket()
julianz- Apr 7, 2026
b469389
Simply the socket OS error handling
julianz- Apr 7, 2026
4341144
Move the wrap() operation into a separate helper
julianz- Apr 7, 2026
0f28843
refactor _from_server_socket exception handler
julianz- Apr 7, 2026
526c291
Move socket acceptance into separate helper
julianz- Apr 7, 2026
63a63ad
Add socket configuration helper
julianz- Apr 7, 2026
45b26ef
Simplify _from_server_socket into two steps
julianz- Apr 7, 2026
9524853
Added IPv4 and IPv6 to spell check dicitonary
julianz- Apr 7, 2026
3b39b17
Added towncrier statement for whole refactor
julianz- Apr 7, 2026
de7bee4
Added tests for refactored code to improve coverage
julianz- Apr 7, 2026
be92e4b
Fix test_tls_client_auth for Windows 2025
julianz- Apr 9, 2026
57031ff
Updated tests for from_server_socket
julianz- Apr 11, 2026
6892f6b
Fix socket leak in _wrap_socket_for_tls on FatalSSLAlert
julianz- Jun 19, 2026
5a26b85
Rename _ignore_socket_oserror to _is_ignorable_socket_error
julianz- Jun 19, 2026
046a870
Replace sentinel tuple returns with exceptions in _prepare_socket
julianz- Jun 19, 2026
0348719
Use full variable names and positional-only args in socket methods
julianz- Jun 19, 2026
6a2809c
Use InterruptedError instead of OSError(EINTR) in transport error test
julianz- Jun 20, 2026
809dd22
Simplify transport error test by removing unnecessary pipe_fd fixture
julianz- Jun 20, 2026
01fde84
Document socket.timeout as OSError subclass and TimeoutError alias
julianz- Jun 20, 2026
050a198
Fix changelog fragment formatting
julianz- Jun 20, 2026
96aa39f
Rephrase _prepare_socket docstring to avoid spell check failures
julianz- Jun 20, 2026
2b63a58
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 131 additions & 75 deletions cheroot/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ def _suppress_socket_io_errors(socket, /):
raise


class _NoConnectionAvailable(Exception):
"""Raised when no connection is ready to be accepted."""


class ConnectionManager:
"""Class which manages HTTPConnection objects.

Expand Down Expand Up @@ -295,88 +299,140 @@ def _remove_invalid_sockets(self):
with _cm.suppress(OSError):
conn.close()

def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
def _setup_conn_addr(self, conn, sock, addr, /):
"""Configure remote address and port for the connection.

Populates the connection object with remote address metadata.
If the address is unavailable following the handshake, this
method detects the socket's address family (IPv4/IPv6)
and provides an appropriate 'any' address placeholder.
"""
# optional values
# Until we do DNS lookups, omit REMOTE_HOST
if addr is None: # sometimes this can happen
# figure out if AF_INET or AF_INET6.
if len(sock.getsockname()) == 2:
# AF_INET
addr = ('0.0.0.0', 0)
else:
# AF_INET6
addr = ('::', 0)
conn.remote_addr = addr[0]
conn.remote_port = addr[1]

def _is_ignorable_socket_error(self, exc, /):
"""Return True if an OSError during socket operations can be ignored."""
if self.server.stats['Enabled']:
self.server.stats['Socket Errors'] += 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably belongs outside a function saying "ignore". Though, maybe the helper could be renamed since it doesn't actually ignore anything but checks for expected exceptions.

How about something like def _is_suppressable_exc(exc: OSError, /) -> bool:? OTOH, could you check if specific OSError subclasses aren't being raised? Maybe, we could replace checking the low-level codes with suppressing the corresponding specific exceptions like InterruptedError and similar?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, though I prefer the slightly more intuitive _is_ignorable_socket_error() as the name. I added the / but since none of the other functions have type annotations I left those out β€” maybe that's better done in a separate PR?

err_code = exc.args[0]

# EINTR should occur when a signal is received during accept();
# retry the call. See https://github.com/cherrypy/cherrypy/issues/707.
is_eintr = err_code in errors.socket_error_eintr

# Just try again. See https://github.com/cherrypy/cherrypy/issues/479.
is_nonblocking = err_code in errors.socket_errors_nonblocking

# Our socket was closed. See https://github.com/cherrypy/cherrypy/issues/686.
is_ignored = err_code in errors.socket_errors_to_ignore

return is_eintr or is_nonblocking or is_ignored

def _wrap_socket_for_tls(self, raw_socket, addr):
"""Handle the TLS wrap and log specific error responses.

On success returns e.g. (SSLSocket, {'SSL_PROTOCOL': 'TLSv1.3', ...}).
On failure closes the socket and raises ConnectionError.
"""
try:
s, addr = server_socket.accept()
if self.server.stats['Enabled']:
self.server.stats['Accepts'] += 1
prevent_socket_inheritance(s)
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:
# FIXME: WPS505 -- too many nested blocks
try: # noqa: WPS505
s, ssl_env = self.server.ssl_adapter.wrap(s)
except errors.FatalSSLAlert as tls_connection_drop_error:
self.server.error_log(
f'Client {addr!s} lost β€” peer dropped the TLS '
'connection suddenly, during handshake: '
f'{tls_connection_drop_error!s}',
)
return None
except errors.NoSSLError as http_over_https_err:
self.server.error_log(
f'Client {addr!s} attempted to speak plain HTTP into '
'a TCP connection configured for TLS-only traffic β€” '
'trying to send back a plain HTTP error response: '
f'{http_over_https_err!s}',
)
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)

if not isinstance(self.server.bind_addr, (str, bytes)):
# optional values
# Until we do DNS lookups, omit REMOTE_HOST
if addr is None: # sometimes this can happen
# figure out if AF_INET or AF_INET6.
if len(s.getsockname()) == 2:
# AF_INET
addr = ('0.0.0.0', 0)
else:
# AF_INET6
addr = ('::', 0)
conn.remote_addr = addr[0]
conn.remote_port = addr[1]

conn.ssl_env = ssl_env
return conn
return self.server.ssl_adapter.wrap(raw_socket)
except errors.FatalSSLAlert as tls_connection_drop_error:
self.server.error_log(
f'Client {addr!s} lost β€” peer dropped the TLS '
'connection suddenly, during handshake: '
f'{tls_connection_drop_error!s}',
)
except errors.NoSSLError as http_over_https_err:
self.server.error_log(
f'Client {addr!s} attempted to speak plain HTTP into '
'a TCP connection configured for TLS-only traffic β€” '
'trying to send back a plain HTTP error response: '
f'{http_over_https_err!s}',
)
self._send_bad_request_plain_http_error(raw_socket)

# If we hit either exception, close the socket and signal failure

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not do this in the fatal ssl alert case too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

with _cm.suppress(OSError):
raw_socket.close()
raise ConnectionError

def _accept_conn(self, server_socket):
"""Accept the connection."""
try:
sock, addr = server_socket.accept()
# socket.timeout: OSError subclass (3.3+), TimeoutError alias (3.10+)
except socket.timeout:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to leave a note that this is a subclass of OSError since Python 3.3 and since Python 3.10, it's a deprecated alias of TimeoutError.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# The only reason for the timeout in start() is so we can
# notice keyboard interrupts on Win32, which don't interrupt
# accept() by default
return None
except OSError as ex:
if self.server.stats['Enabled']:
self.server.stats['Socket Errors'] += 1
if ex.args[0] in errors.socket_error_eintr:
# I *think* this is right. EINTR should occur when a signal
# is received during the accept() call; all docs say retry
# the call, and I *think* I'm reading it right that Python
# will then go ahead and poll for and handle the signal
# elsewhere. See
# https://github.com/cherrypy/cherrypy/issues/707.
return None
if ex.args[0] in errors.socket_errors_nonblocking:
# Just try again. See
# https://github.com/cherrypy/cherrypy/issues/479.
return None
if ex.args[0] in errors.socket_errors_to_ignore:
# Our socket was closed.
# See https://github.com/cherrypy/cherrypy/issues/686.
except OSError as exc:
if self._is_ignorable_socket_error(exc):
return None
raise

if self.server.stats['Enabled']:
self.server.stats['Accepts'] += 1

return sock, addr

def _configure_socket(self, sock):
"""Apply standard settings to a new socket."""
prevent_socket_inheritance(sock)
if hasattr(sock, 'settimeout'):
sock.settimeout(self.server.timeout)

def _prepare_socket(self, server_socket):
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
"""Handle physical accept and TLS negotiation.

Returns a 4-tuple of the accepted socket (possibly TLS-wrapped),
the remote address and port, a callable that wraps the socket for
buffered reading, and a dict of TLS metadata (empty if not using TLS).
Raises ``_NoConnectionAvailable`` if no connection is ready to accept.
Raises ``ConnectionError`` if TLS negotiation fails.
"""
result = self._accept_conn(server_socket)
if result is None:
raise _NoConnectionAvailable
sock, addr = result

self._configure_socket(sock)

makefile = MakeFile
ssl_env = {}

if self.server.ssl_adapter is not None:
sock, ssl_env = self._wrap_socket_for_tls(sock, addr)
makefile = self.server.ssl_adapter.makefile
# Re-apply our timeout since we may have a new socket object
if hasattr(sock, 'settimeout'):
sock.settimeout(self.server.timeout)

return sock, addr, makefile, ssl_env

def _from_server_socket(self, server_socket):
"""Orchestrate the creation of a connection from a server socket."""
try:
sock, addr, makefile, ssl_env = self._prepare_socket(server_socket)
except (_NoConnectionAvailable, ConnectionError):
return None

# Initialize the Connection object
conn = self.server.ConnectionClass(self.server, sock, makefile)
conn.ssl_env = ssl_env

if not isinstance(self.server.bind_addr, (str, bytes)):
self._setup_conn_addr(conn, sock, addr)

return conn

def close(self):
"""Close all monitored connections."""
for _, conn in self._selector.connections:
Expand Down
Loading
Loading