From a807a07a5938990f88839298d6c6c5a58241d3a8 Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Thu, 6 Mar 2025 15:25:11 +0100 Subject: [PATCH] Always try to deserialize message in case of Docker transport errors Before this commit, if the status code was 4xx or 500, we tried to read the errors object, consuming the http entity. When we tried to deserialize the message, the http entity was already consumed, an IOException has been thrown and null is returned for the message. Now, we read the content in a byte[] and deserialize the errors and the message from that. This ensures that we can read both the errors and the message. Closes gh-44628 --- .../docker/transport/HttpClientTransport.java | 43 +++++++++++++------ .../transport/HttpClientTransportTests.java | 15 ++++++- .../docker/transport/message-and-errors.json | 15 +++++++ 3 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/transport/message-and-errors.json diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransport.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransport.java index c0387e12243..7ea5f00f3d2 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransport.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransport.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,7 +34,6 @@ import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.io.entity.AbstractHttpEntity; -import org.apache.hc.core5.http.message.StatusLine; import org.springframework.boot.buildpack.platform.io.Content; import org.springframework.boot.buildpack.platform.io.IOConsumer; @@ -49,6 +48,7 @@ import org.springframework.util.StringUtils; * @author Phillip Webb * @author Mike Smithson * @author Scott Frederick + * @author Moritz Halbritter */ abstract class HttpClientTransport implements HttpTransport { @@ -147,12 +147,12 @@ abstract class HttpClientTransport implements HttpTransport { ClassicHttpResponse response = this.client.executeOpen(this.host, request, null); int statusCode = response.getCode(); if (statusCode >= 400 && statusCode <= 500) { - HttpEntity entity = response.getEntity(); - Errors errors = (statusCode != 500) ? getErrorsFromResponse(entity) : null; - Message message = getMessageFromResponse(entity); - StatusLine statusLine = new StatusLine(response); + byte[] content = readContent(response); + response.close(); + Errors errors = (statusCode != 500) ? deserializeErrors(content) : null; + Message message = deserializeMessage(content); throw new DockerEngineException(this.host.toHostString(), request.getUri(), statusCode, - statusLine.getReasonPhrase(), errors, message); + response.getReasonPhrase(), errors, message); } return new HttpClientResponse(response); } @@ -161,19 +161,38 @@ abstract class HttpClientTransport implements HttpTransport { } } - private Errors getErrorsFromResponse(HttpEntity entity) { + private byte[] readContent(ClassicHttpResponse response) throws IOException { + HttpEntity entity = response.getEntity(); + if (entity == null) { + return null; + } + try (InputStream stream = entity.getContent()) { + if (stream == null) { + return null; + } + return stream.readAllBytes(); + } + } + + private Errors deserializeErrors(byte[] content) { + if (content == null) { + return null; + } try { - return SharedObjectMapper.get().readValue(entity.getContent(), Errors.class); + return SharedObjectMapper.get().readValue(content, Errors.class); } catch (IOException ex) { return null; } } - private Message getMessageFromResponse(HttpEntity entity) { + private Message deserializeMessage(byte[] content) { + if (content == null) { + return null; + } try { - return (entity.getContent() != null) - ? SharedObjectMapper.get().readValue(entity.getContent(), Message.class) : null; + Message message = SharedObjectMapper.get().readValue(content, Message.class); + return (message.getMessage() != null) ? message : null; } catch (IOException ex) { return null; diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransportTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransportTests.java index 40bd62e2a4e..508c0e54c94 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransportTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/transport/HttpClientTransportTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,7 @@ import static org.mockito.BDDMockito.then; * @author Phillip Webb * @author Mike Smithson * @author Scott Frederick + * @author Moritz Halbritter */ @ExtendWith(MockitoExtension.class) class HttpClientTransportTests { @@ -309,6 +310,18 @@ class HttpClientTransportTests { }); } + @Test + void shouldReturnErrorsAndMessage() throws IOException { + givenClientWillReturnResponse(); + given(this.entity.getContent()).willReturn(getClass().getResourceAsStream("message-and-errors.json")); + given(this.response.getCode()).willReturn(404); + assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri)) + .satisfies((ex) -> { + assertThat(ex.getErrors()).hasSize(2); + assertThat(ex.getResponseMessage().getMessage()).contains("test message"); + }); + } + @Test void executeWhenClientThrowsIOExceptionRethrowsAsDockerException() throws IOException { given(this.client.executeOpen(any(HttpHost.class), any(HttpUriRequest.class), isNull())) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/transport/message-and-errors.json b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/transport/message-and-errors.json new file mode 100644 index 00000000000..ec5357ab093 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/transport/message-and-errors.json @@ -0,0 +1,15 @@ +{ + "message": "test message", + "errors": [ + { + "code": "TEST1", + "message": "Test One", + "detail": 123 + }, + { + "code": "TEST2", + "message": "Test Two", + "detail": "fail" + } + ] +}