From 738097def2407166ff9d58c25836180f95eda77d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 23 Nov 2018 13:55:50 +0100 Subject: [PATCH] DefaultResponseErrorHandler detects non-standard error code as well Issue: SPR-17439 --- .../org/springframework/http/HttpStatus.java | 42 ++++++++++--- .../client/DefaultResponseErrorHandler.java | 22 ++++++- ...ltResponseErrorHandlerHttpStatusTests.java | 17 +++--- .../DefaultResponseErrorHandlerTests.java | 60 +++++++++++++++++-- 4 files changed, 115 insertions(+), 26 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/HttpStatus.java b/spring-web/src/main/java/org/springframework/http/HttpStatus.java index d7d67b8224..1d8e861415 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpStatus.java +++ b/spring-web/src/main/java/org/springframework/http/HttpStatus.java @@ -522,7 +522,6 @@ public enum HttpStatus { return status; } - /** * Resolve the given status code to an {@code HttpStatus}, if possible. * @param statusCode the HTTP status code (potentially non-standard) @@ -565,18 +564,45 @@ public enum HttpStatus { return this.value; } - public static Series valueOf(int status) { - int seriesCode = status / 100; + /** + * Return the enum constant of this type with the corresponding series. + * @param status a standard HTTP status enum value + * @return the enum constant of this type with the corresponding series + * @throws IllegalArgumentException if this enum has no corresponding constant + */ + public static Series valueOf(HttpStatus status) { + return valueOf(status.value); + } + + /** + * Return the enum constant of this type with the corresponding series. + * @param statusCode the HTTP status code (potentially non-standard) + * @return the enum constant of this type with the corresponding series + * @throws IllegalArgumentException if this enum has no corresponding constant + */ + public static Series valueOf(int statusCode) { + Series series = resolve(statusCode); + if (series == null) { + throw new IllegalArgumentException("No matching constant for [" + statusCode + "]"); + } + return series; + } + + /** + * Resolve the given status code to an {@code HttpStatus.Series}, if possible. + * @param statusCode the HTTP status code (potentially non-standard) + * @return the corresponding {@code Series}, or {@code null} if not found + * @since 5.1.3 + */ + @Nullable + public static Series resolve(int statusCode) { + int seriesCode = statusCode / 100; for (Series series : values()) { if (series.value == seriesCode) { return series; } } - throw new IllegalArgumentException("No matching constant for [" + status + "]"); - } - - public static Series valueOf(HttpStatus status) { - return valueOf(status.value); + return null; } } 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 536e733967..744dfcc66b 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 @@ -48,8 +48,9 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler { */ @Override public boolean hasError(ClientHttpResponse response) throws IOException { - HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode()); - return (statusCode != null && hasError(statusCode)); + int rawStatusCode = response.getRawStatusCode(); + HttpStatus statusCode = HttpStatus.resolve(rawStatusCode); + return (statusCode != null ? hasError(statusCode) : hasError(rawStatusCode)); } /** @@ -58,7 +59,7 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler { * {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or * {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}. * Can be overridden in subclasses. - * @param statusCode the HTTP status code + * @param statusCode the HTTP status code as enum value * @return {@code true} if the response has an error; {@code false} otherwise */ protected boolean hasError(HttpStatus statusCode) { @@ -66,6 +67,21 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler { statusCode.series() == HttpStatus.Series.SERVER_ERROR); } + /** + * Template method called from {@link #hasError(ClientHttpResponse)}. + *

The default implementation checks if the given status code is + * {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or + * {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}. + * Can be overridden in subclasses. + * @param unknownStatusCode the HTTP status code as raw value + * @return {@code true} if the response has an error; {@code false} otherwise + * @since 4.3.21 + */ + protected boolean hasError(int unknownStatusCode) { + HttpStatus.Series series = HttpStatus.Series.resolve(unknownStatusCode); + return (series == HttpStatus.Series.CLIENT_ERROR || series == HttpStatus.Series.SERVER_ERROR); + } + /** * Delegates to {@link #handleError(ClientHttpResponse, HttpStatus)} with the response status code. */ diff --git a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerHttpStatusTests.java b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerHttpStatusTests.java index 00ca4a6e6a..a85769d6e3 100644 --- a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerHttpStatusTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerHttpStatusTests.java @@ -28,7 +28,6 @@ import org.springframework.http.client.ClientHttpResponse; import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; -import static org.mockito.Mockito.mock; import static org.springframework.http.HttpStatus.*; /** @@ -66,7 +65,7 @@ public class DefaultResponseErrorHandlerHttpStatusTests { public HttpStatus httpStatus; @Parameterized.Parameter(1) - public Class expectedExceptionClass; + public Class expectedExceptionClass; private final DefaultResponseErrorHandler handler = new DefaultResponseErrorHandler(); @@ -74,7 +73,13 @@ public class DefaultResponseErrorHandlerHttpStatusTests { @Test - public void handleErrorIOException() throws Exception { + public void hasErrorTrue() throws Exception { + given(this.response.getRawStatusCode()).willReturn(this.httpStatus.value()); + assertTrue(this.handler.hasError(this.response)); + } + + @Test + public void handleErrorException() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.TEXT_PLAIN); @@ -91,10 +96,4 @@ public class DefaultResponseErrorHandlerHttpStatusTests { } } - @Test - public void hasErrorTrue() throws Exception { - given(this.response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value()); - assertTrue(handler.hasError(this.response)); - } - } 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 4731d43661..0391e74021 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 @@ -65,7 +65,7 @@ public class DefaultResponseErrorHandlerTests { given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value()); given(response.getStatusText()).willReturn("Not Found"); given(response.getHeaders()).willReturn(headers); - given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes("UTF-8"))); + given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8))); try { handler.handleError(response); @@ -101,8 +101,20 @@ public class DefaultResponseErrorHandlerTests { handler.handleError(response); } + @Test // SPR-16108 + public void hasErrorForUnknownStatusCode() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_PLAIN); + + given(response.getRawStatusCode()).willReturn(999); + given(response.getStatusText()).willReturn("Custom status code"); + given(response.getHeaders()).willReturn(headers); + + assertFalse(handler.hasError(response)); + } + @Test(expected = UnknownHttpStatusCodeException.class) // SPR-9406 - public void unknownStatusCode() throws Exception { + public void handleErrorUnknownStatusCode() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.TEXT_PLAIN); @@ -113,16 +125,52 @@ public class DefaultResponseErrorHandlerTests { handler.handleError(response); } - @Test // SPR-16108 - public void hasErrorForUnknownStatusCode() throws Exception { + @Test // SPR-17461 + public void hasErrorForCustomClientError() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.TEXT_PLAIN); - given(response.getRawStatusCode()).willReturn(999); + given(response.getRawStatusCode()).willReturn(499); given(response.getStatusText()).willReturn("Custom status code"); given(response.getHeaders()).willReturn(headers); - assertFalse(handler.hasError(response)); + assertTrue(handler.hasError(response)); + } + + @Test(expected = UnknownHttpStatusCodeException.class) + public void handleErrorForCustomClientError() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_PLAIN); + + given(response.getRawStatusCode()).willReturn(499); + given(response.getStatusText()).willReturn("Custom status code"); + given(response.getHeaders()).willReturn(headers); + + handler.handleError(response); + } + + @Test // SPR-17461 + public void hasErrorForCustomServerError() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_PLAIN); + + given(response.getRawStatusCode()).willReturn(599); + given(response.getStatusText()).willReturn("Custom status code"); + given(response.getHeaders()).willReturn(headers); + + assertTrue(handler.hasError(response)); + } + + @Test(expected = UnknownHttpStatusCodeException.class) + public void handleErrorForCustomServerError() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_PLAIN); + + given(response.getRawStatusCode()).willReturn(599); + given(response.getStatusText()).willReturn("Custom status code"); + given(response.getHeaders()).willReturn(headers); + + handler.handleError(response); } @Test // SPR-16604