diff --git a/docker/api/image.py b/docker/api/image.py index 85109473..5f34d362 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -1,8 +1,11 @@ +import itertools +import json import logging import os from .. import auth, errors, utils from ..constants import DEFAULT_DATA_CHUNK_SIZE +from ..utils.json_stream import json_stream log = logging.getLogger(__name__) @@ -433,6 +436,29 @@ class ImageApiMixin: return self._result(response) + @staticmethod + def _raise_if_error(chunk, response): + """ + Raise an exception if the given chunk of the JSON server response is a + dictionary and contains an "error" field. Otherwise, return the chunk + as-is. + + Args: + chunk (object): A chunk of the server response. + response (Response): The full server response. This will be attached + to the exception in the event that chunk indicates an error. + + Returns: + (object): The input chunk. + + Raises: + :py:class:`docker.errors.APIError` + If the chunk of the server response contains an error message. + """ + if isinstance(chunk, dict) and 'error' in chunk: + raise errors.APIError(chunk['error'], response=response) + return chunk + def push(self, repository, tag=None, stream=False, auth_config=None, decode=False): """ @@ -494,8 +520,25 @@ class ImageApiMixin: self._raise_for_status(response) + # The server response might have status code 200 (OK) even though the + # push operation has failed. To detect errors, inspect each JSON chunk + # of the server response and check if an "error" entry is present. + # See: https://github.com/docker/docker-py/issues/3277 if stream: - return self._stream_helper(response, decode=decode) + if decode: + return (self._raise_if_error(chunk, response) for chunk in + self._stream_helper(response, decode=True)) + else: + result_stream, internal_stream = itertools.tee( + self._stream_helper(response, decode=False)) + for chunk_json in json_stream(internal_stream): + self._raise_if_error(chunk_json, response) + return result_stream + + for chunk_str in response.text.splitlines(): + chunk_json = json.loads(chunk_str) + if 'error' in chunk_json: + raise errors.APIError(chunk_json['error'], response=response) return self._result(response) diff --git a/docker/utils/json_stream.py b/docker/utils/json_stream.py index 41d25920..c7fd46fa 100644 --- a/docker/utils/json_stream.py +++ b/docker/utils/json_stream.py @@ -1,4 +1,3 @@ -import json import json.decoder from ..errors import StreamParseError @@ -37,30 +36,12 @@ def json_stream(stream): This handles streams which are inconsistently buffered (some entries may be newline delimited, and others are not). """ - return split_buffer(stream, json_splitter, json_decoder.decode) - - -def line_splitter(buffer, separator='\n'): - index = buffer.find(str(separator)) - if index == -1: - return None - return buffer[:index + 1], buffer[index + 1:] - - -def split_buffer(stream, splitter=None, decoder=lambda a: a): - """Given a generator which yields strings and a splitter function, - joins all input, splits on the separator and yields each chunk. - Unlike string.split(), each chunk includes the trailing - separator, except for the last one if none was found on the end - of the input. - """ - splitter = splitter or line_splitter buffered = '' for data in stream_as_text(stream): buffered += data while True: - buffer_split = splitter(buffered) + buffer_split = json_splitter(buffered) if buffer_split is None: break @@ -69,6 +50,13 @@ def split_buffer(stream, splitter=None, decoder=lambda a: a): if buffered: try: - yield decoder(buffered) + yield json_decoder.decode(buffered) except Exception as e: raise StreamParseError(e) from e + + +def line_splitter(buffer: str, separator='\n'): + index = buffer.find(str(separator)) + if index == -1: + return None + return buffer[:index + 1], buffer[index + 1:] diff --git a/tests/unit/api_image_test.py b/tests/unit/api_image_test.py index 148109d3..321421b0 100644 --- a/tests/unit/api_image_test.py +++ b/tests/unit/api_image_test.py @@ -271,6 +271,33 @@ class ImageTest(BaseAPIClientTest): timeout=DEFAULT_TIMEOUT_SECONDS ) + + def test_push_image_with_auth_error(self): + auth_config = { + 'username': "test_user", + 'password': "test_password", + 'serveraddress': "test_server", + } + encoded_auth = auth.encode_header(auth_config) + with pytest.raises(docker.errors.APIError, match='bad auth'): + self.client.push( + fake_api.FAKE_IMAGE_NAME_ERROR, tag=fake_api.FAKE_TAG_NAME, + auth_config=auth_config + ) + + fake_request.assert_called_with( + 'POST', + f"{url_prefix}images/test_image_error/push", + params={ + 'tag': fake_api.FAKE_TAG_NAME, + }, + data='{}', + headers={'Content-Type': 'application/json', + 'X-Registry-Auth': encoded_auth}, + stream=False, + timeout=DEFAULT_TIMEOUT_SECONDS + ) + def test_push_image_stream(self): with mock.patch('docker.auth.resolve_authconfig', fake_resolve_authconfig): @@ -288,6 +315,59 @@ class ImageTest(BaseAPIClientTest): timeout=DEFAULT_TIMEOUT_SECONDS ) + + def test_push_image_stream_with_auth(self): + auth_config = { + 'username': "test_user", + 'password': "test_password", + 'serveraddress': "test_server", + } + encoded_auth = auth.encode_header(auth_config) + self.client.push( + fake_api.FAKE_IMAGE_NAME, tag=fake_api.FAKE_TAG_NAME, + auth_config=auth_config, stream=True + ) + + fake_request.assert_called_with( + 'POST', + f"{url_prefix}images/test_image/push", + params={ + 'tag': fake_api.FAKE_TAG_NAME, + }, + data='{}', + headers={'Content-Type': 'application/json', + 'X-Registry-Auth': encoded_auth}, + stream=True, + timeout=DEFAULT_TIMEOUT_SECONDS + ) + + + def test_push_image_stream_with_auth_error(self): + auth_config = { + 'username': "test_user", + 'password': "test_password", + 'serveraddress': "test_server", + } + encoded_auth = auth.encode_header(auth_config) + with pytest.raises(docker.errors.APIError, match='bad auth'): + self.client.push( + fake_api.FAKE_IMAGE_NAME_ERROR, tag=fake_api.FAKE_TAG_NAME, + auth_config=auth_config, stream=True + ) + + fake_request.assert_called_with( + 'POST', + f"{url_prefix}images/test_image_error/push", + params={ + 'tag': fake_api.FAKE_TAG_NAME, + }, + data='{}', + headers={'Content-Type': 'application/json', + 'X-Registry-Auth': encoded_auth}, + stream=True, + timeout=DEFAULT_TIMEOUT_SECONDS + ) + def test_tag_image(self): self.client.tag(fake_api.FAKE_IMAGE_ID, fake_api.FAKE_REPO_NAME) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index 3ce127b3..2882ad5c 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -31,6 +31,8 @@ def response(status_code=200, content='', headers=None, reason=None, elapsed=0, request=None, raw=None): res = requests.Response() res.status_code = status_code + if isinstance(content, str): + content = content.encode('ascii') if not isinstance(content, bytes): content = json.dumps(content).encode('ascii') res._content = content @@ -42,6 +44,30 @@ def response(status_code=200, content='', headers=None, reason=None, elapsed=0, return res +def stream_response(status_code=200, content='', **kwargs): + if isinstance(content, str): + content_bytes = content.encode('ascii') + elif isinstance(content, bytes): + content_bytes = content + else: + content_bytes = json.dumps(content).encode('ascii') + + body = io.BytesIO(content_bytes) + + # mock a stream interface + raw_resp = urllib3.HTTPResponse(body=body) + raw_resp._fp.chunked = True + raw_resp._fp.chunk_left = len(body.getvalue()) - 1 + raw_resp._fp.seek(0) + + return response( + status_code=status_code, + content=content_bytes, + raw=raw_resp, + **kwargs, + ) + + def fake_resolve_authconfig(authconfig, registry=None, *args, **kwargs): return None @@ -59,6 +85,8 @@ def fake_resp(method, url, *args, **kwargs): if not key: raise Exception(f'{method} {url}') status_code, content = fake_api.fake_responses[key]() + if kwargs.get("stream", False): + return stream_response(status_code=status_code, content=content) return response(status_code=status_code, content=content) diff --git a/tests/unit/fake_api.py b/tests/unit/fake_api.py index 03e53cc6..e44ed5bb 100644 --- a/tests/unit/fake_api.py +++ b/tests/unit/fake_api.py @@ -1,3 +1,5 @@ +import json + from docker import constants from . import fake_stat @@ -9,6 +11,7 @@ FAKE_IMAGE_ID = 'sha256:fe7a8fc91d3f17835cbb3b86a1c60287500ab01a53bc79c4497d09f0 FAKE_EXEC_ID = 'b098ec855f10434b5c7c973c78484208223a83f663ddaefb0f02a242840cb1c7' FAKE_NETWORK_ID = '1999cfb42e414483841a125ade3c276c3cb80cb3269b14e339354ac63a31b02c' FAKE_IMAGE_NAME = 'test_image' +FAKE_IMAGE_NAME_ERROR = 'test_image_error' FAKE_TARBALL_PATH = '/path/to/tarball' FAKE_REPO_NAME = 'repo' FAKE_TAG_NAME = 'tag' @@ -359,6 +362,12 @@ def post_fake_push(): return status_code, response +def post_fake_push_error(): + status_code = 200 + response = '{"status": "intermediate update"}\r\n{"error": "bad auth"}\r\n' + return status_code, response + + def post_fake_build_container(): status_code = 200 response = {'Id': FAKE_CONTAINER_ID} @@ -603,6 +612,8 @@ fake_responses = { get_fake_insert_image, f'{prefix}/{CURRENT_VERSION}/images/test_image/push': post_fake_push, + f'{prefix}/{CURRENT_VERSION}/images/test_image_error/push': + post_fake_push_error, f'{prefix}/{CURRENT_VERSION}/commit': post_fake_commit, f'{prefix}/{CURRENT_VERSION}/containers/create':