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