Skip to content

Commit 2aa4578

Browse files
webknjazjulianz-
authored andcommitted
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
1 parent c94db0c commit 2aa4578

File tree

6 files changed

+30
-7
lines changed

6 files changed

+30
-7
lines changed

cheroot/connections.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
327327
wfile = mf(s, 'wb', io.DEFAULT_BUFFER_SIZE)
328328
try:
329329
wfile.write(''.join(buf).encode('ISO-8859-1'))
330+
wfile.flush()
331+
wfile.close()
330332
except OSError as ex:
331333
if ex.args[0] not in errors.socket_errors_to_ignore:
332334
raise

cheroot/makefile.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,29 @@ def write(self, val, *args, **kwargs):
6868
self.bytes_written += len(val)
6969
return res
7070

71+
def _flush_unlocked(self):
72+
"""
73+
Flush buffered output to the underlying socket.
74+
75+
We override this method because when a client sends plaintext HTTP
76+
to a TLS-only port, SSL detects a bad handshake and may
77+
invalidate the underlying socket. At that point, the socket
78+
may not be writable at the moment of flush. Attempting to write
79+
(e.g., sending a 400 Bad Request) may succeed or raise OSError.
80+
This override prevents OSError from propagating and clears the buffer
81+
to prevent further write attempts.
82+
"""
83+
try:
84+
super()._flush_unlocked()
85+
except OSError:
86+
# The socket is already closed or otherwise unusable (e.g. TLS
87+
# fatal alert triggered by invalid pre-handshake plaintext).
88+
#
89+
# Clearing the buffer prevents additional write attempts during
90+
# object finalization (BufferedWriter.__del__), avoiding noisy
91+
# OSError reports at interpreter shutdown.
92+
self._write_buf.clear()
93+
7194

7295
def MakeFile(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE):
7396
"""File object attached to a socket object."""

cheroot/server.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,6 +1370,7 @@ def _handle_no_ssl(self, req):
13701370
'this server only speaks HTTPS on this port.'
13711371
)
13721372
req.simple_response('400 Bad Request', msg)
1373+
self.wfile.flush()
13731374
self.linger = True
13741375

13751376
def _conditional_error(self, req, response):

cheroot/test/test_ssl.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,6 @@ def test_https_over_http_error(http_server, ip_addr):
713713
pytest.param(ANY_INTERFACE_IPV6, marks=missing_ipv6),
714714
),
715715
)
716-
@pytest.mark.flaky(reruns=3, reruns_delay=2)
717716
# pylint: disable-next=too-many-positional-arguments
718717
def test_http_over_https_error(
719718
http_request_timeout,
@@ -726,12 +725,6 @@ def test_http_over_https_error(
726725
tls_certificate_private_key_pem_path,
727726
):
728727
"""Ensure that connecting over HTTP to HTTPS port is handled."""
729-
# disable some flaky tests
730-
# https://github.com/cherrypy/cheroot/issues/225
731-
issue_225 = IS_MACOS and adapter_type == 'builtin'
732-
if issue_225:
733-
pytest.xfail('Test fails in Travis-CI')
734-
735728
if IS_LINUX:
736729
expected_error_code, expected_error_text = (
737730
104,
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fixed socket errors when clients send HTTP to HTTPS-only ports.
2+
3+
-- by :user:`julianz-`

docs/spelling_wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ netloc
4040
noqa
4141
PIL
4242
pipelining
43+
plaintext
4344
positionally
4445
pre
4546
preconfigure

0 commit comments

Comments
 (0)