Skip to content

Commit 4cbef27

Browse files
jhoellerjhoeller
committed
Consistent tolerance of unknown HTTP status codes behind RestTemplate
Issue: SPR-15978
1 parent 215e5f5 commit 4cbef27

File tree

14 files changed

+154
-150
lines changed

14 files changed

+154
-150
lines changed

‎spring-web/src/main/java/org/springframework/http/HttpStatus.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.http;
1818

19+
import org.springframework.lang.Nullable;
20+
1921
/**
2022
* Enumeration of HTTP status codes.
2123
*
@@ -499,12 +501,28 @@ public String toString() {
499501
* @throws IllegalArgumentException if this enum has no constant for the specified numeric value
500502
*/
501503
public static HttpStatus valueOf(int statusCode) {
504+
HttpStatus status = resolve(statusCode);
505+
if (status == null) {
506+
throw new IllegalArgumentException("No matching constant for [" + statusCode + "]");
507+
}
508+
return status;
509+
}
510+
511+
512+
/**
513+
* Resolve the given status code to an {@code HttpStatus}, if possible.
514+
* @param statusCode the HTTP status code (potentially non-standard)
515+
* @return the corresponding {@code HttpStatus}, or {@code null} if not found
516+
* @since 5.0
517+
*/
518+
@Nullable
519+
public static HttpStatus resolve(int statusCode) {
502520
for (HttpStatus status : values()) {
503521
if (status.value == statusCode) {
504522
return status;
505523
}
506524
}
507-
throw new IllegalArgumentException("No matching constant for [" + statusCode + "]");
525+
return null;
508526
}
509527

510528

‎spring-web/src/main/java/org/springframework/http/client/ClientHttpResponse.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,13 +38,19 @@ public interface ClientHttpResponse extends HttpInputMessage, Closeable {
3838
* Return the HTTP status code of the response.
3939
* @return the HTTP status as an HttpStatus enum value
4040
* @throws IOException in case of I/O errors
41+
* @throws IllegalArgumentException in case of an unknown HTTP status code
42+
* @see HttpStatus#valueOf(int)
4143
*/
4244
HttpStatus getStatusCode() throws IOException;
4345

4446
/**
45-
* Return the HTTP status code of the response as integer
47+
* Return the HTTP status code (potentially non-standard and not
48+
* resolvable through the {@link HttpStatus} enum) as an integer.
4649
* @return the HTTP status as an integer
4750
* @throws IOException in case of I/O errors
51+
* @since 3.1.1
52+
* @see #getStatusCode()
53+
* @see HttpStatus#resolve(int)
4854
*/
4955
int getRawStatusCode() throws IOException;
5056

‎spring-web/src/main/java/org/springframework/web/client/AsyncRestTemplate.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ private void logResponseStatus(HttpMethod method, URI url, ClientHttpResponse re
523523
if (logger.isDebugEnabled()) {
524524
try {
525525
logger.debug("Async " + method.name() + " request for \"" + url + "\" resulted in " +
526-
response.getStatusCode() + " (" + response.getStatusText() + ")");
526+
response.getRawStatusCode() + " (" + response.getStatusText() + ")");
527527
}
528528
catch (IOException ex) {
529529
// ignore
@@ -535,7 +535,7 @@ private void handleResponseError(HttpMethod method, URI url, ClientHttpResponse
535535
if (logger.isWarnEnabled()) {
536536
try {
537537
logger.warn("Async " + method.name() + " request for \"" + url + "\" resulted in " +
538-
response.getStatusCode() + " (" + response.getStatusText() + "); invoking error handler");
538+
response.getRawStatusCode() + " (" + response.getStatusText() + "); invoking error handler");
539539
}
540540
catch (IOException ex) {
541541
// ignore

‎spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java‎

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,46 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
4646
*/
4747
@Override
4848
public boolean hasError(ClientHttpResponse response) throws IOException {
49-
return hasError(getHttpStatusCode(response));
49+
HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode());
50+
return (statusCode != null && hasError(statusCode));
5051
}
5152

5253
/**
53-
* This default implementation throws a {@link HttpClientErrorException} if the response status code
54+
* Template method called from {@link #hasError(ClientHttpResponse)}.
55+
* <p>The default implementation checks if the given status code is
56+
* {@link HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
57+
* {@link HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
58+
* Can be overridden in subclasses.
59+
* @param statusCode the HTTP status code
60+
* @return {@code true} if the response has an error; {@code false} otherwise
61+
*/
62+
protected boolean hasError(HttpStatus statusCode) {
63+
return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
64+
statusCode.series() == HttpStatus.Series.SERVER_ERROR);
65+
}
66+
67+
/**
68+
* Delegates to {@link #handleError(ClientHttpResponse, HttpStatus)} with the response status code.
69+
*/
70+
@Override
71+
public void handleError(ClientHttpResponse response) throws IOException {
72+
HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode());
73+
if (statusCode == null) {
74+
throw new UnknownHttpStatusCodeException(response.getRawStatusCode(), response.getStatusText(),
75+
response.getHeaders(), getResponseBody(response), getCharset(response));
76+
}
77+
handleError(response, statusCode);
78+
}
79+
80+
/**
81+
* Handle the error in the given response with the given resolved status code.
82+
* <p>This default implementation throws a {@link HttpClientErrorException} if the response status code
5483
* is {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}, a {@link HttpServerErrorException}
5584
* if it is {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR},
5685
* and a {@link RestClientException} in other cases.
86+
* @since 5.0
5787
*/
58-
@Override
59-
public void handleError(ClientHttpResponse response) throws IOException {
60-
HttpStatus statusCode = getHttpStatusCode(response);
88+
protected void handleError(ClientHttpResponse response, HttpStatus statusCode) throws IOException {
6189
switch (statusCode.series()) {
6290
case CLIENT_ERROR:
6391
throw new HttpClientErrorException(statusCode, response.getStatusText(),
@@ -66,7 +94,8 @@ public void handleError(ClientHttpResponse response) throws IOException {
6694
throw new HttpServerErrorException(statusCode, response.getStatusText(),
6795
response.getHeaders(), getResponseBody(response), getCharset(response));
6896
default:
69-
throw new RestClientException("Unknown status code [" + statusCode + "]");
97+
throw new UnknownHttpStatusCodeException(statusCode.value(), response.getStatusText(),
98+
response.getHeaders(), getResponseBody(response), getCharset(response));
7099
}
71100
}
72101

@@ -79,43 +108,16 @@ public void handleError(ClientHttpResponse response) throws IOException {
79108
* @throws UnknownHttpStatusCodeException in case of an unknown status code
80109
* that cannot be represented with the {@link HttpStatus} enum
81110
* @since 4.3.8
111+
* @deprecated as of 5.0, in favor of {@link #handleError(ClientHttpResponse, HttpStatus)}
82112
*/
113+
@Deprecated
83114
protected HttpStatus getHttpStatusCode(ClientHttpResponse response) throws IOException {
84-
try {
85-
return response.getStatusCode();
86-
}
87-
catch (IllegalArgumentException ex) {
115+
HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode());
116+
if (statusCode == null) {
88117
throw new UnknownHttpStatusCodeException(response.getRawStatusCode(), response.getStatusText(),
89118
response.getHeaders(), getResponseBody(response), getCharset(response));
90119
}
91-
}
92-
93-
/**
94-
* Template method called from {@link #hasError(ClientHttpResponse)}.
95-
* <p>The default implementation checks if the given status code is
96-
* {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR}
97-
* or {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
98-
* Can be overridden in subclasses.
99-
* @param statusCode the HTTP status code
100-
* @return {@code true} if the response has an error; {@code false} otherwise
101-
* @see #getHttpStatusCode(ClientHttpResponse)
102-
*/
103-
protected boolean hasError(HttpStatus statusCode) {
104-
return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
105-
statusCode.series() == HttpStatus.Series.SERVER_ERROR);
106-
}
107-
108-
/**
109-
* Determine the charset of the response (for inclusion in a status exception).
110-
* @param response the response to inspect
111-
* @return the associated charset, or {@code null} if none
112-
* @since 4.3.8
113-
*/
114-
@Nullable
115-
protected Charset getCharset(ClientHttpResponse response) {
116-
HttpHeaders headers = response.getHeaders();
117-
MediaType contentType = headers.getContentType();
118-
return (contentType != null ? contentType.getCharset() : null);
120+
return statusCode;
119121
}
120122

121123
/**
@@ -135,4 +137,17 @@ protected byte[] getResponseBody(ClientHttpResponse response) {
135137
return new byte[0];
136138
}
137139

140+
/**
141+
* Determine the charset of the response (for inclusion in a status exception).
142+
* @param response the response to inspect
143+
* @return the associated charset, or {@code null} if none
144+
* @since 4.3.8
145+
*/
146+
@Nullable
147+
protected Charset getCharset(ClientHttpResponse response) {
148+
HttpHeaders headers = response.getHeaders();
149+
MediaType contentType = headers.getContentType();
150+
return (contentType != null ? contentType.getCharset() : null);
151+
}
152+
138153
}

‎spring-web/src/main/java/org/springframework/web/client/ExtractingResponseErrorHandler.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,15 @@ else if (this.seriesMapping.containsKey(statusCode.series())) {
132132
}
133133

134134
@Override
135-
public void handleError(ClientHttpResponse response) throws IOException {
136-
HttpStatus statusCode = getHttpStatusCode(response);
135+
public void handleError(ClientHttpResponse response, HttpStatus statusCode) throws IOException {
137136
if (this.statusMapping.containsKey(statusCode)) {
138137
extract(this.statusMapping.get(statusCode), response);
139138
}
140139
else if (this.seriesMapping.containsKey(statusCode.series())) {
141140
extract(this.seriesMapping.get(statusCode.series()), response);
142141
}
143142
else {
144-
super.handleError(response);
143+
super.handleError(response, statusCode);
145144
}
146145
}
147146

‎spring-web/src/main/java/org/springframework/web/client/MessageBodyClientHttpResponseWrapper.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ public MessageBodyClientHttpResponseWrapper(ClientHttpResponse response) throws
5858
* @throws IOException in case of I/O errors
5959
*/
6060
public boolean hasMessageBody() throws IOException {
61-
HttpStatus responseStatus = this.getStatusCode();
62-
if (responseStatus.is1xxInformational() || responseStatus == HttpStatus.NO_CONTENT ||
63-
responseStatus == HttpStatus.NOT_MODIFIED) {
61+
HttpStatus status = HttpStatus.resolve(getRawStatusCode());
62+
if (status != null && (status.is1xxInformational() || status == HttpStatus.NO_CONTENT ||
63+
status == HttpStatus.NOT_MODIFIED)) {
6464
return false;
6565
}
66-
else if (getHeaders().getContentLength() == 0) {
66+
if (getHeaders().getContentLength() == 0) {
6767
return false;
6868
}
6969
return true;

‎spring-web/src/main/java/org/springframework/web/client/RestTemplate.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -969,10 +969,10 @@ public ResponseEntityResponseExtractor(@Nullable Type responseType) {
969969
public ResponseEntity<T> extractData(ClientHttpResponse response) throws IOException {
970970
if (this.delegate != null) {
971971
T body = this.delegate.extractData(response);
972-
return new ResponseEntity<>(body, response.getHeaders(), response.getStatusCode());
972+
return ResponseEntity.status(response.getRawStatusCode()).headers(response.getHeaders()).body(body);
973973
}
974974
else {
975-
return new ResponseEntity<>(response.getHeaders(), response.getStatusCode());
975+
return ResponseEntity.status(response.getRawStatusCode()).headers(response.getHeaders()).build();
976976
}
977977
}
978978
}

‎spring-web/src/main/java/org/springframework/web/client/UnknownHttpStatusCodeException.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,12 +38,12 @@ public class UnknownHttpStatusCodeException extends RestClientResponseException
3838
* {@link HttpStatus}, status text, and response body content.
3939
* @param rawStatusCode the raw status code value
4040
* @param statusText the status text
41-
* @param responseHeaders the response headers, may be {@code null}
42-
* @param responseBody the response body content, may be {@code null}
43-
* @param responseCharset the response body charset, may be {@code null}
41+
* @param responseHeaders the response headers (may be {@code null})
42+
* @param responseBody the response body content (may be {@code null})
43+
* @param responseCharset the response body charset (may be {@code null})
4444
*/
45-
public UnknownHttpStatusCodeException(int rawStatusCode, String statusText,
46-
@Nullable HttpHeaders responseHeaders, @Nullable byte[] responseBody,@Nullable Charset responseCharset) {
45+
public UnknownHttpStatusCodeException(int rawStatusCode, String statusText, @Nullable HttpHeaders responseHeaders,
46+
@Nullable byte[] responseBody, @Nullable Charset responseCharset) {
4747

4848
super("Unknown status code [" + String.valueOf(rawStatusCode) + "]" + " " + statusText,
4949
rawStatusCode, statusText, responseHeaders, responseBody, responseCharset);

‎spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ public class DefaultResponseErrorHandlerTests {
4343

4444
@Test
4545
public void hasErrorTrue() throws Exception {
46-
given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
46+
given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
4747
assertTrue(handler.hasError(response));
4848
}
4949

5050
@Test
5151
public void hasErrorFalse() throws Exception {
52-
given(response.getStatusCode()).willReturn(HttpStatus.OK);
52+
given(response.getRawStatusCode()).willReturn(HttpStatus.OK.value());
5353
assertFalse(handler.hasError(response));
5454
}
5555

@@ -58,7 +58,7 @@ public void handleError() throws Exception {
5858
HttpHeaders headers = new HttpHeaders();
5959
headers.setContentType(MediaType.TEXT_PLAIN);
6060

61-
given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
61+
given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
6262
given(response.getStatusText()).willReturn("Not Found");
6363
given(response.getHeaders()).willReturn(headers);
6464
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes("UTF-8")));
@@ -77,7 +77,7 @@ public void handleErrorIOException() throws Exception {
7777
HttpHeaders headers = new HttpHeaders();
7878
headers.setContentType(MediaType.TEXT_PLAIN);
7979

80-
given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
80+
given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
8181
given(response.getStatusText()).willReturn("Not Found");
8282
given(response.getHeaders()).willReturn(headers);
8383
given(response.getBody()).willThrow(new IOException());
@@ -90,7 +90,7 @@ public void handleErrorNullResponse() throws Exception {
9090
HttpHeaders headers = new HttpHeaders();
9191
headers.setContentType(MediaType.TEXT_PLAIN);
9292

93-
given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
93+
given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
9494
given(response.getStatusText()).willReturn("Not Found");
9595
given(response.getHeaders()).willReturn(headers);
9696

@@ -102,7 +102,6 @@ public void unknownStatusCode() throws Exception {
102102
HttpHeaders headers = new HttpHeaders();
103103
headers.setContentType(MediaType.TEXT_PLAIN);
104104

105-
given(response.getStatusCode()).willThrow(new IllegalArgumentException("No matching constant for 999"));
106105
given(response.getRawStatusCode()).willReturn(999);
107106
given(response.getStatusText()).willReturn("Custom status code");
108107
given(response.getHeaders()).willReturn(headers);

0 commit comments

Comments
 (0)