From 0540de5b1ea5a9472803fbf4fdd629885a58d662 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 17 Nov 2025 04:27:26 +0100 Subject: [PATCH] Handle socket errors when sending HTTP-over-HTTPS error responses When a client sends plaintext HTTP to a TLS-only port, the SSL layer detects the invalid handshake and may close the underlying socket. The server attempts to send a 400 Bad Request error response, but the socket may already be closed, causing OSError during the flush. With pyOpenSSL, the response usually succeeds. With the builtin SSL adapter, the socket may be closed before the write can occur. This fix overrides `_flush_unlocked()` in `StreamWriter` to catch `OSError` and clear the buffer. This allows: - The explicit flush to fail gracefully when sending the 400 response - Object finalization (`__del__`) to complete without errors --- cheroot/connections.py | 2 ++ cheroot/makefile.py | 20 ++++++++++++++++++++ cheroot/server.py | 1 + cheroot/test/test_ssl.py | 7 ------- docs/changelog-fragments.d/802.bugfix.rst | 3 +++ docs/spelling_wordlist.txt | 1 + 6 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 docs/changelog-fragments.d/802.bugfix.rst diff --git a/cheroot/connections.py b/cheroot/connections.py index 04df0d42a3..189f30026c 100644 --- a/cheroot/connections.py +++ b/cheroot/connections.py @@ -327,6 +327,8 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME wfile = mf(s, 'wb', io.DEFAULT_BUFFER_SIZE) try: wfile.write(''.join(buf).encode('ISO-8859-1')) + wfile.flush() + wfile.close() except OSError as ex: if ex.args[0] not in errors.socket_errors_to_ignore: raise diff --git a/cheroot/makefile.py b/cheroot/makefile.py index f5780a1ede..a06e3a4414 100644 --- a/cheroot/makefile.py +++ b/cheroot/makefile.py @@ -68,6 +68,26 @@ def write(self, val, *args, **kwargs): self.bytes_written += len(val) return res + def _flush_unlocked(self): + """ + Flush buffered output to the underlying socket. + + We override this method because when a client sends plaintext HTTP + to a TLS-only port, SSL detects a bad handshake and may + invalidate the underlying socket. At that point, the socket + may not be writable. Attempting to write (e.g., sending a + 400 Bad Request) may succeed or raise OSError. This override + prevents OSError from propagating. + """ + try: + super()._flush_unlocked() + except OSError: + # The socket is already closed or otherwise unusable (e.g. TLS + # fatal alert triggered by invalid handshake). + # Clearing the buffer prevents the error from happening + # again during cleanup. + self._write_buf.clear() + def MakeFile(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE): """File object attached to a socket object.""" diff --git a/cheroot/server.py b/cheroot/server.py index 27e9173b15..1cc496ad7c 100644 --- a/cheroot/server.py +++ b/cheroot/server.py @@ -1370,6 +1370,7 @@ def _handle_no_ssl(self, req): 'this server only speaks HTTPS on this port.' ) req.simple_response('400 Bad Request', msg) + self.wfile.flush() self.linger = True def _conditional_error(self, req, response): diff --git a/cheroot/test/test_ssl.py b/cheroot/test/test_ssl.py index 881b5ae990..1c3b8da487 100644 --- a/cheroot/test/test_ssl.py +++ b/cheroot/test/test_ssl.py @@ -713,7 +713,6 @@ def test_https_over_http_error(http_server, ip_addr): pytest.param(ANY_INTERFACE_IPV6, marks=missing_ipv6), ), ) -@pytest.mark.flaky(reruns=3, reruns_delay=2) # pylint: disable-next=too-many-positional-arguments def test_http_over_https_error( http_request_timeout, @@ -726,12 +725,6 @@ def test_http_over_https_error( tls_certificate_private_key_pem_path, ): """Ensure that connecting over HTTP to HTTPS port is handled.""" - # disable some flaky tests - # https://github.com/cherrypy/cheroot/issues/225 - issue_225 = IS_MACOS and adapter_type == 'builtin' - if issue_225: - pytest.xfail('Test fails in Travis-CI') - if IS_LINUX: expected_error_code, expected_error_text = ( 104, diff --git a/docs/changelog-fragments.d/802.bugfix.rst b/docs/changelog-fragments.d/802.bugfix.rst new file mode 100644 index 0000000000..d13097a262 --- /dev/null +++ b/docs/changelog-fragments.d/802.bugfix.rst @@ -0,0 +1,3 @@ +Fixed socket errors when clients send HTTP to HTTPS-only ports. + +-- by :user:`julianz-` diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 1e1022c82f..6ab6243e0c 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -40,6 +40,7 @@ netloc noqa PIL pipelining +plaintext positionally pre preconfigure