Skip to content

Commit ed64521

Browse files
committed
Refactor _from_server_socket()
This function had become highly nested and complex, triggering linter warnings (C901, WPS505). Decomposed the logic into several smaller private methods: - ``_wrap_tls_socket()`` - ``_create_conn()`` - ``_handle_socket_error()`` - ``_setup_conn_addr()``
1 parent 2ffb0ba commit ed64521

File tree

1 file changed

+109
-60
lines changed

1 file changed

+109
-60
lines changed

cheroot/connections.py

Lines changed: 109 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -295,87 +295,136 @@ def _remove_invalid_sockets(self):
295295
with _cm.suppress(OSError):
296296
conn.close()
297297

298-
def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
298+
def _setup_conn_addr(self, conn, s, addr):
299+
"""Configure remote address and port for the connection."""
300+
# Optional values
301+
# Until we do DNS lookups, omit REMOTE_HOST
302+
if addr is None:
303+
# Fallback for sockets that don't return an address on accept
304+
# figure out if AF_INET or AF_INET6.
305+
if len(s.getsockname()) == 2:
306+
# AF_INET
307+
addr = ('0.0.0.0', 0)
308+
else:
309+
# AF_INET6
310+
addr = ('::', 0)
311+
conn.remote_addr = addr[0]
312+
conn.remote_port = addr[1]
313+
314+
def _handle_socket_error(self, ex, s=None):
315+
"""Handle OSErrors and determine if they should be ignored."""
316+
if self.server.stats['Enabled']:
317+
self.server.stats['Socket Errors'] += 1
318+
319+
if s:
320+
with _cm.suppress(OSError):
321+
s.close()
322+
323+
err_code = ex.args[0] if ex.args else None
324+
ignored_groups = (
325+
errors.socket_error_eintr,
326+
# I *think* this is right. EINTR should occur when a signal
327+
# is received during the accept() call; all docs say retry
328+
# the call, and I *think* I'm reading it right that Python
329+
# will then go ahead and poll for and handle the signal
330+
# elsewhere. See
331+
# https://github.com/cherrypy/cherrypy/issues/707.
332+
errors.socket_errors_nonblocking,
333+
# Just try again. See
334+
# https://github.com/cherrypy/cherrypy/issues/479.
335+
errors.socket_errors_to_ignore,
336+
# Our socket was closed.
337+
# See https://github.com/cherrypy/cherrypy/issues/686.
338+
)
339+
if any(err_code in group for group in ignored_groups):
340+
return
341+
342+
raise ex
343+
344+
def _wrap_tls_socket(self, s, addr):
345+
"""Handle the TLS wrap and log specific error responses.
346+
347+
on success returns e.g. (SSLSocket, {'SSL_PROTOCOL': 'TLSv1.3', ...}).
348+
on failure returns None, {}
349+
"""
350+
try:
351+
return self.server.ssl_adapter.wrap(s)
352+
except errors.FatalSSLAlert as tls_connection_drop_error:
353+
self.server.error_log(
354+
f'Client {addr!s} lost — peer dropped the TLS '
355+
'connection suddenly, during handshake: '
356+
f'{tls_connection_drop_error!s}',
357+
)
358+
except errors.NoSSLError as http_over_https_err:
359+
self.server.error_log(
360+
f'Client {addr!s} attempted to speak plain HTTP into '
361+
'a TCP connection configured for TLS-only traffic — '
362+
'trying to send back a plain HTTP error response: '
363+
f'{http_over_https_err!s}',
364+
)
365+
self._send_bad_request_plain_http_error(s)
366+
367+
# If we hit either exception, close the socket and signal failure
368+
with _cm.suppress(OSError):
369+
s.close()
370+
return None, {}
371+
372+
def _create_conn(self, s, addr, ssl_env):
373+
"""Build and configure the Connection object."""
374+
# 1. Determine the makefile type (SSL vs Plain)
375+
mf = MakeFile
376+
if self.server.ssl_adapter is not None:
377+
mf = self.server.ssl_adapter.makefile
378+
379+
# 2. Re-apply timeout specifically for the new SSLSocket object
380+
if hasattr(s, 'settimeout'):
381+
s.settimeout(self.server.timeout)
382+
383+
# 3. Create the actual connection object
384+
conn = self.server.ConnectionClass(self.server, s, mf)
385+
conn.ssl_env = ssl_env
386+
387+
# 4. Configure the remote address/port if it's not a Unix socket
388+
if not isinstance(self.server.bind_addr, (str, bytes)):
389+
self._setup_conn_addr(conn, s, addr)
390+
391+
return conn
392+
393+
def _from_server_socket(self, server_socket):
394+
"""Coordinate socket acceptance and connection initialization."""
299395
try:
300396
s, addr = server_socket.accept()
301397
if self.server.stats['Enabled']:
302398
self.server.stats['Accepts'] += 1
399+
303400
prevent_socket_inheritance(s)
304401
if hasattr(s, 'settimeout'):
305402
s.settimeout(self.server.timeout)
306403

307-
mf = MakeFile
308404
ssl_env = {}
309-
# if ssl cert and key are set, we try to be a secure HTTP server
310405
if self.server.ssl_adapter is not None:
311-
# FIXME: WPS505 -- too many nested blocks
312-
try: # noqa: WPS505
313-
s, ssl_env = self.server.ssl_adapter.wrap(s)
314-
except errors.FatalSSLAlert as tls_connection_drop_error:
315-
self.server.error_log(
316-
f'Client {addr!s} lost — peer dropped the TLS '
317-
'connection suddenly, during handshake: '
318-
f'{tls_connection_drop_error!s}',
319-
)
320-
return None
321-
except errors.NoSSLError as http_over_https_err:
322-
self.server.error_log(
323-
f'Client {addr!s} attempted to speak plain HTTP into '
324-
'a TCP connection configured for TLS-only traffic — '
325-
'trying to send back a plain HTTP error response: '
326-
f'{http_over_https_err!s}',
327-
)
328-
self._send_bad_request_plain_http_error(s)
406+
# try to become a secure server
407+
s, ssl_env = self._wrap_tls_socket(s, addr)
408+
if s is None:
329409
return None
330-
mf = self.server.ssl_adapter.makefile
331-
# Re-apply our timeout since we may have a new socket object
410+
411+
# Re-apply timeout to the new SSLSocket object
332412
if hasattr(s, 'settimeout'):
333413
s.settimeout(self.server.timeout)
334414

335-
conn = self.server.ConnectionClass(self.server, s, mf)
336-
337-
if not isinstance(self.server.bind_addr, (str, bytes)):
338-
# optional values
339-
# Until we do DNS lookups, omit REMOTE_HOST
340-
if addr is None: # sometimes this can happen
341-
# figure out if AF_INET or AF_INET6.
342-
if len(s.getsockname()) == 2:
343-
# AF_INET
344-
addr = ('0.0.0.0', 0)
345-
else:
346-
# AF_INET6
347-
addr = ('::', 0)
348-
conn.remote_addr = addr[0]
349-
conn.remote_port = addr[1]
350-
351-
conn.ssl_env = ssl_env
352-
return conn
415+
return self._create_conn(s, addr, ssl_env)
353416

354417
except socket.timeout:
355418
# The only reason for the timeout in start() is so we can
356419
# notice keyboard interrupts on Win32, which don't interrupt
357420
# accept() by default
358421
return None
359422
except OSError as ex:
360-
if self.server.stats['Enabled']:
361-
self.server.stats['Socket Errors'] += 1
362-
if ex.args[0] in errors.socket_error_eintr:
363-
# I *think* this is right. EINTR should occur when a signal
364-
# is received during the accept() call; all docs say retry
365-
# the call, and I *think* I'm reading it right that Python
366-
# will then go ahead and poll for and handle the signal
367-
# elsewhere. See
368-
# https://github.com/cherrypy/cherrypy/issues/707.
369-
return None
370-
if ex.args[0] in errors.socket_errors_nonblocking:
371-
# Just try again. See
372-
# https://github.com/cherrypy/cherrypy/issues/479.
373-
return None
374-
if ex.args[0] in errors.socket_errors_to_ignore:
375-
# Our socket was closed.
376-
# See https://github.com/cherrypy/cherrypy/issues/686.
377-
return None
378-
raise
423+
# if socket.accept() fails s may not be defined
424+
# or if the handshake fails it may exist but
425+
# will need to be closed.
426+
# pass it over to error handler if it exists
427+
return self._handle_socket_error(ex, locals().get('s'))
379428

380429
def close(self):
381430
"""Close all monitored connections."""

0 commit comments

Comments
 (0)