Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConnectionError if Docker Engine sends early error response #3278

Open
adamjseitz opened this issue Aug 9, 2024 · 0 comments
Open

ConnectionError if Docker Engine sends early error response #3278

adamjseitz opened this issue Aug 9, 2024 · 0 comments

Comments

@adamjseitz
Copy link

adamjseitz commented Aug 9, 2024

Describe the bug

When a client sends a request to the Docker Engine that is invalid in some way, the Docker Engine can send an error response and close the connection before the client is finished sending. This results in a cryptic requests.exceptions.ConnectionError exception and provides no way to retrieve the error response sent from the Docker Engine.

To Reproduce

This can be reproduced by calling APIClient.import_image with a large .tar file and a invalid repository reference such as invalid::latest.

$ cat big_docker_import.py
import tarfile
import io
import docker

# Write 100 MB of data to big.tar
with tarfile.open("./big.tar", "w") as f:
    data = 100 * 1024 * 1024 * b'\xff'
    info = tarfile.TarInfo('big.bin')
    info.size = len(data)
    f.addfile(info, fileobj=io.BytesIO(data))

# Import with an invalid reference
api_client = docker.APIClient(base_url="unix:///var/run/docker.sock")
api_client.import_image(
    "./big.tar",
    repository="invalid::latest",
    changes=[],
)

$ python3 big_docker_import.py
Traceback (most recent call last):
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 790, in urlopen
    response = self._make_request(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 496, in _make_request
    conn.request(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connection.py", line 402, in request
    self.send(chunk)
  File "/usr/lib/python3.8/http/client.py", line 972, in send
    self.sock.sendall(data)
ConnectionResetError: [Errno 104] Connection reset by peer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/adapters.py", line 667, in send
    resp = conn.urlopen(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 844, in urlopen
    retries = retries.increment(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/util/retry.py", line 470, in increment
    raise reraise(type(error), error, _stacktrace)
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/util/util.py", line 38, in reraise
    raise value.with_traceback(tb)
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 790, in urlopen
    response = self._make_request(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 496, in _make_request
    conn.request(
  File "/home/aseitz/.local/lib/python3.8/site-packages/urllib3/connection.py", line 402, in request
    self.send(chunk)
  File "/usr/lib/python3.8/http/client.py", line 972, in send
    self.sock.sendall(data)
urllib3.exceptions.ProtocolError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "big_docker_import.py", line 14, in <module>
    api_client.import_image(
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/image.py", line 143, in import_image
    self._post(
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/utils/decorators.py", line 44, in inner
    return f(self, *args, **kwargs)
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/client.py", line 242, in _post
    return self.post(url, **self._set_request_timeout(kwargs))
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/sessions.py", line 637, in post
    return self.request("POST", url, data=data, json=json, **kwargs)
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/sessions.py", line 589, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/sessions.py", line 703, in send
    r = adapter.send(request, **kwargs)
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/adapters.py", line 682, in send
    raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))

Expected behavior

The correct behavior is demonstrated by making a similar request, but with an empty .tar file. In this case, we correctly get a docker.errors.APIError that contains the invalid reference format message from the Docker Engine.

$ cat empty_docker_import.py
import pathlib
import docker

# Create an empty .tar
pathlib.Path("./empty.tar").touch()

# Import with an invalid reference
api_client = docker.APIClient(base_url="unix:///var/run/docker.sock")
api_client.import_image(
    "./empty.tar",
    repository="invalid::latest",
    changes=[],
)
$ python3 empty_docker_import.py
Traceback (most recent call last):
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/client.py", line 275, in _raise_for_status
    response.raise_for_status()
  File "/home/aseitz/.local/lib/python3.8/site-packages/requests/models.py", line 1024, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: http+docker://localhost/v1.46/images/create?repo=invalid%3A%3Alatest&fromSrc=-

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "empty_docker_import.py", line 9, in <module>
    api_client.import_image(
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/image.py", line 142, in import_image
    return self._result(
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/client.py", line 281, in _result
    self._raise_for_status(response)
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/api/client.py", line 277, in _raise_for_status
    raise create_api_error_from_http_exception(e) from e
  File "/home/aseitz/.local/lib/python3.8/site-packages/docker/errors.py", line 39, in create_api_error_from_http_exception
    raise cls(e, response=response, explanation=explanation) from e
docker.errors.APIError: 400 Client Error for http+docker://localhost/v1.46/images/create?repo=invalid%3A%3Alatest&fromSrc=-: Bad Request ("invalid reference format")

Environment

  • OS version: Ubuntu 20.04
  • SDK version: 7.1.0
  • Docker version: 27.1.1
  • Python version: 3.8.10
  • urllib3 version: 2.0.3

Additional context

The Python Docker SDK uses a custom UnixHTTPConnection adapter with the requests library to use HTTP over the Docker socket: https://github.com/docker/docker-py/blob/main/docker/transport/unixconn.py#L13

The HTTP server (the Docker engine in this case) should be allowed to send a response and terminate the connection before we are done writing our request, and we should be able to read that response after catching the ConnectionResetError.

urllib3, which requests uses for its HTTP implementation, does do this:

        try:
            conn.request(
                ...
            )

        # 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:
            pass

https://github.com/urllib3/urllib3/blob/main/src/urllib3/connectionpool.py#L506

However, the exception that is thrown in our case is ConnectionResetError instead of the BrokenPipeError that urllib3 expects.

My theory here is that because the underlying socket is a Unix domain socket, which urllib3 is not expecting (remember we create the Unix domain socket in the UnixHTTPConnection adapter), it raises a different exception type in the analogous situation: ConnectionResetError instead of BrokenPipeError.

Making the following change in a local version of urllib3 resolves the problem:

diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py
index 2479405b..99ad8286 100644
--- a/src/urllib3/connectionpool.py
+++ b/src/urllib3/connectionpool.py
@@ -507,7 +507,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         # 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:
+        except (BrokenPipeError, ConnectionResetError):
             pass
         except OSError as e:
             # MacOS/Linux

It's not so clear how to fix the Docker SDK. Should urllib3 be handling ConnectionResetError (i.e., is this an upstream bug)? Or if this only occurs because of the overridden socket type, how can we modify the adapter to try to get the HTTP response when the error has occurred?

I believe this issue describes what is happening in #2950, #2836, and maybe #2526.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant