From 211581e6ea9bb1aa4226c0c4c3ea0f8373459c50 Mon Sep 17 00:00:00 2001
From: Sviatoslav Sydorenko <wk@sydorenko.org.ua>
Date: Mon, 30 Nov 2020 18:51:58 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Suppress=20legit=20OS=20errors?=
 =?UTF-8?q?=20on=20socket=20shutdown?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When closing a socket we normally want to terminate the transport-
level connection by sending a TCP FIN packet over the wire. This works
great if the client is willing to perform a 3-way handshake but
sometimes the client-side just drops the connection by sending an RST
instead of a FIN. In this case, our server-side `socket.shutdown()`
call will raise an OSError (socket.error on Python 2) or its
derivatives. Which is perfectly fine and it's alright for us to ignore
those.

The kernel socket close helper rewrite introduced a behavior of only
suppressing ENOTCONN in v8.4.8 (the case when the client is no longer
with us) but it left own a few other cases that may be happening too.

This change fixes that by extending the list of errors that are to be
suppressed. Here's what's handled from now on:
  * ENOTCONN — client is no longer connected
  * EPIPE — write on a pipe while the other end has been closed
  * ESHUTDOWN — write on a socket which has been shutdown for writing
  * ECONNRESET — connection is reset by the peer

Fixes #341

Refs:
  * https://en.wikipedia.org/wiki/Transmission_Control_Protocol#Connection_termination
  * http://cs.baylor.edu/~donahoo/practical/CSockets/TCPRST.pdf
  * https://github.com/cherrypy/cheroot/issues/341#issuecomment-735884889
---
 cheroot/errors.py | 22 ++++++++++++++++++++++
 cheroot/server.py |  6 +++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/cheroot/errors.py b/cheroot/errors.py
index 4395e56528..414d5cbce5 100644
--- a/cheroot/errors.py
+++ b/cheroot/errors.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 """Collection of exceptions raised and/or processed by Cheroot."""
 
 from __future__ import absolute_import, division, print_function
@@ -56,3 +57,24 @@ def plat_specific_errors(*errnames):
 if sys.platform == 'darwin':
     socket_errors_to_ignore.extend(plat_specific_errors('EPROTOTYPE'))
     socket_errors_nonblocking.extend(plat_specific_errors('EPROTOTYPE'))
+
+
+acceptable_sock_shutdown_error_codes = {
+    errno.ENOTCONN,
+    errno.EPIPE, errno.ESHUTDOWN,  # corresponds to BrokenPipeError in Python 3
+    errno.ECONNRESET,  # corresponds to ConnectionResetError in Python 3
+}
+"""Errors that may happen during the connection close sequence.
+
+* ENOTCONN — client is no longer connected
+* EPIPE — write on a pipe while the other end has been closed
+* ESHUTDOWN — write on a socket which has been shutdown for writing
+* ECONNRESET — connection is reset by the peer
+
+Ref: https://github.com/cherrypy/cheroot/issues/341#issuecomment-735884889
+"""
+
+try:  # py3
+    broken_connection_exceptions = (BrokenPipeError, ConnectionResetError)
+except NameError:  # py2
+    broken_connection_exceptions = ()
diff --git a/cheroot/server.py b/cheroot/server.py
index a5c31b2e65..8ecf9265a2 100644
--- a/cheroot/server.py
+++ b/cheroot/server.py
@@ -75,7 +75,6 @@
 import platform
 import contextlib
 import threading
-import errno
 
 try:
     from functools import lru_cache
@@ -1478,9 +1477,10 @@ def _close_kernel_socket(self):
 
         try:
             shutdown(socket.SHUT_RDWR)  # actually send a TCP FIN
+        except errors.broken_connection_exceptions:
+            pass
         except socket.error as e:
-            # Suppress "client is no longer connected"
-            if e.errno != errno.ENOTCONN:
+            if e.errno not in errors.acceptable_sock_shutdown_error_codes:
                 raise