Swallow BrokenPipeError when writing request body

In the case the server legitimately closes a connection but we are still
sending data (e.g. sending a POST request), the old code prevents the
response to be retrieved.

With this change, we avoid a broken pipe in the request write process
to be able to capture the HTTP response.

Co-authored-by: Rober Morales-Chaparro <rober.morales@gmail.com>
Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>
Co-authored-by: hodbn <hodbn@users.noreply.github.com>
Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>

src/urllib3/connection.py

@@ -30,6 +30,15 @@ class ConnectionError(Exception):
         pass
 
 
+try:  # Python 3:
+    # Not a no-op, we're adding this to the namespace so it can be imported.
+    BrokenPipeError = BrokenPipeError
+except NameError:  # Python 2:
+
+    class BrokenPipeError(Exception):
+        pass
+
+
 from .exceptions import (
     NewConnectionError,
     ConnectTimeoutError,

src/urllib3/connectionpool.py

@@ -34,6 +34,7 @@
     VerifiedHTTPSConnection,
     HTTPException,
     BaseSSLError,
+    BrokenPipeError,
 )
 from .request import RequestMethods
 from .response import HTTPResponse
@@ -386,10 +387,28 @@ def _make_request(
 
         # conn.request() calls httplib.*.request, not the method in
         # urllib3.request. It also calls makefile (recv) on the socket.
-        if chunked:
-            conn.request_chunked(method, url, **httplib_request_kw)
-        else:
-            conn.request(method, url, **httplib_request_kw)
+        try:
+            if chunked:
+                conn.request_chunked(method, url, **httplib_request_kw)
+            else:
+                conn.request(method, url, **httplib_request_kw)
+
+        # We are swallowing BrokenPipeError (errno.EPIPE) since the server is
+        # legitimately able to close the connection after sending a valid response.
+        # With this behaviour, the received response is still readable.
+        except BrokenPipeError:
+            # Python 3
+            pass
+        except IOError as e:
+            # Python 2 and macOS/Linux
+            # EPIPE and ESHUTDOWN are BrokenPipeError on Python 2, and EPROTOTYPE is needed on macOS
+            # https://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/
+            if e.errno not in {
+                errno.EPIPE,
+                errno.ESHUTDOWN,
+                errno.EPROTOTYPE,
+            }:
+                raise
 
         # Reset the timeout for the recv() on the socket
         read_timeout = timeout_obj.read_timeout

test/__init__.py

@@ -148,7 +148,7 @@ def notWindows(test):
 
     @six.wraps(test)
     def wrapper(*args, **kwargs):
-        msg = "{} is flaky on Windows".format(test.__name__)
+        msg = "{name} does not run on Windows".format(name=test.__name__)
         if platform.system() == "Windows":
             pytest.skip(msg)
         return test(*args, **kwargs)

test/with_dummyserver/test_socketlevel.py

@@ -1,6 +1,7 @@
 # TODO: Break this module up into pieces. Maybe group by functionality tested
 # rather than the socket level-ness of it.
 from urllib3 import HTTPConnectionPool, HTTPSConnectionPool
+from urllib3.connection import HTTPConnection
 from urllib3.poolmanager import proxy_from_url
 from urllib3.connection import _get_default_user_agent
 from urllib3.exceptions import (
@@ -56,6 +57,7 @@ class MimeToolMessage(object):
     LONG_TIMEOUT,
     notPyPy2,
     notSecureTransport,
+    notWindows,
     resolvesLocalhostFQDN,
 )
 
@@ -1809,6 +1811,49 @@ def socket_handler(listener):
             assert pool.num_connections == 1
 
 
+class TestBrokenPipe(SocketDummyServerTestCase):
+    @notWindows
+    def test_ignore_broken_pipe_errors(self, monkeypatch):
+        # On Windows an aborted connection raises an error on
+        # attempts to read data out of a socket that's been closed.
+        sock_shut = Event()
+        orig_connect = HTTPConnection.connect
+        # a buffer that will cause two sendall calls
+        buf = "a" * 1024 * 1024 * 4
+
+        def connect_and_wait(*args, **kw):
+            ret = orig_connect(*args, **kw)
+            assert sock_shut.wait(5)
+            return ret
+
+        def socket_handler(listener):
+            for i in range(2):
+                sock = listener.accept()[0]
+                sock.send(
+                    b"HTTP/1.1 404 Not Found\r\n"
+                    b"Connection: close\r\n"
+                    b"Content-Length: 10\r\n"
+                    b"\r\n"
+                    b"xxxxxxxxxx"
+                )
+                sock.shutdown(socket.SHUT_RDWR)
+                sock_shut.set()
+                sock.close()
+
+        monkeypatch.setattr(HTTPConnection, "connect", connect_and_wait)
+        self._start_server(socket_handler)
+        with HTTPConnectionPool(self.host, self.port) as pool:
+            r = pool.request("POST", "/", body=buf)
+            assert r.status == 404
+            assert r.headers["content-length"] == "10"
+            assert r.data == b"xxxxxxxxxx"
+
+            r = pool.request("POST", "/admin", chunked=True, body=buf)
+            assert r.status == 404
+            assert r.headers["content-length"] == "10"
+            assert r.data == b"xxxxxxxxxx"
+
+
 class TestMultipartResponse(SocketDummyServerTestCase):
     def test_multipart_assert_header_parsing_no_defects(self):
         def socket_handler(listener):