From 2fb13d410d3938a8c4e875247e5e50eee1406f4b Mon Sep 17 00:00:00 2001 From: Andrew Woodbury Date: Tue, 25 Feb 2020 18:28:21 -0500 Subject: [PATCH] Include response body in UnknownHttpStatusCodeException Spring Framework 5.2.2 introduced a regression in DefaultResponseErrorHandler.handleError(ClientHttpResponse) Specifically, for use cases where the InputStream had already been consumed by the first invocation of getResponseBody(), the second invocation of getResponseBody() resulted in the response body being absent in the created UnknownHttpStatusCodeException. This commit fixes this by invoking getResponseBody() only once in DefaultResponseErrorHandler.handleError(ClientHttpReponse) in order to reuse the retrieved response body for creating the exception message and as a separate argument to the UnknownHttpStatusCodeException constructor. Closes gh-24595 --- .../client/DefaultResponseErrorHandler.java | 5 +- .../DefaultResponseErrorHandlerTests.java | 46 +++++++++++++++---- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java index ff72eb1e465d..2f6422e8a9ae 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java @@ -102,12 +102,13 @@ protected boolean hasError(int unknownStatusCode) { public void handleError(ClientHttpResponse response) throws IOException { HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode()); if (statusCode == null) { + byte[] body = getResponseBody(response); String message = getErrorMessage( response.getRawStatusCode(), response.getStatusText(), - getResponseBody(response), getCharset(response)); + body, getCharset(response)); throw new UnknownHttpStatusCodeException(message, response.getRawStatusCode(), response.getStatusText(), - response.getHeaders(), getResponseBody(response), getCharset(response)); + response.getHeaders(), body, getCharset(response)); } handleError(response, statusCode); } diff --git a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java index c5c05c48e426..30d321fa5658 100644 --- a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java @@ -32,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -160,15 +161,29 @@ public void hasErrorForCustomClientError() throws Exception { @Test public void handleErrorForCustomClientError() throws Exception { + int statusCode = 499; + String statusText = "Custom status code"; + HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.TEXT_PLAIN); - given(response.getRawStatusCode()).willReturn(499); - given(response.getStatusText()).willReturn("Custom status code"); + String responseBody = "Hello World"; + TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)); + + given(response.getRawStatusCode()).willReturn(statusCode); + given(response.getStatusText()).willReturn(statusText); given(response.getHeaders()).willReturn(headers); + given(response.getBody()).willReturn(body); - assertThatExceptionOfType(UnknownHttpStatusCodeException.class).isThrownBy(() -> - handler.handleError(response)); + Throwable throwable = catchThrowable(() -> handler.handleError(response)); + + // validate exception + assertThat(throwable).isInstanceOf(UnknownHttpStatusCodeException.class); + UnknownHttpStatusCodeException actualUnknownHttpStatusCodeException = (UnknownHttpStatusCodeException) throwable; + assertThat(actualUnknownHttpStatusCodeException.getRawStatusCode()).isEqualTo(statusCode); + assertThat(actualUnknownHttpStatusCodeException.getStatusText()).isEqualTo(statusText); + assertThat(actualUnknownHttpStatusCodeException.getResponseHeaders()).isEqualTo(headers); + assertThat(actualUnknownHttpStatusCodeException.getResponseBodyAsString()).isEqualTo(responseBody); } @Test // SPR-17461 @@ -185,15 +200,29 @@ public void hasErrorForCustomServerError() throws Exception { @Test public void handleErrorForCustomServerError() throws Exception { + int statusCode = 599; + String statusText = "Custom status code"; + HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.TEXT_PLAIN); - given(response.getRawStatusCode()).willReturn(599); - given(response.getStatusText()).willReturn("Custom status code"); + String responseBody = "Hello World"; + TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)); + + given(response.getRawStatusCode()).willReturn(statusCode); + given(response.getStatusText()).willReturn(statusText); given(response.getHeaders()).willReturn(headers); + given(response.getBody()).willReturn(body); - assertThatExceptionOfType(UnknownHttpStatusCodeException.class).isThrownBy(() -> - handler.handleError(response)); + Throwable throwable = catchThrowable(() -> handler.handleError(response)); + + // validate exception + assertThat(throwable).isInstanceOf(UnknownHttpStatusCodeException.class); + UnknownHttpStatusCodeException actualUnknownHttpStatusCodeException = (UnknownHttpStatusCodeException) throwable; + assertThat(actualUnknownHttpStatusCodeException.getRawStatusCode()).isEqualTo(statusCode); + assertThat(actualUnknownHttpStatusCodeException.getStatusText()).isEqualTo(statusText); + assertThat(actualUnknownHttpStatusCodeException.getResponseHeaders()).isEqualTo(headers); + assertThat(actualUnknownHttpStatusCodeException.getResponseBodyAsString()).isEqualTo(responseBody); } @Test // SPR-16604 @@ -212,7 +241,6 @@ public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception { assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8)).isEqualTo("Hello World"); } - private static class TestByteArrayInputStream extends ByteArrayInputStream { private boolean closed;