Skip to content

Commit

Permalink
Include response body in UnknownHttpStatusCodeException
Browse files Browse the repository at this point in the history
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
  • Loading branch information
awoodbury authored and sbrannen committed Mar 17, 2020
1 parent 5e1e689 commit 2fb13d4
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
Expand Up @@ -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);
}
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand Down

0 comments on commit 2fb13d4

Please sign in to comment.