Raise RestClientException for unknown status codes

HttpStatus cannot be created with an unknown status code. If a server
returns a status code that's not in the HttpStatus enum values, an
IllegalArgumentException is raised. Rather than allowing it to
propagate as such, this change ensures the actual exception raised is
a RestClientException.

Issue: SPR-9406
Backport-Issue: SPR-9502

build-spring-framework/resources/changelog.txt

@@ -19,6 +19,7 @@ Changes in version 3.1.2 (2012-06-??)
 * add Jackson 2 HttpMessageConverter and View types
 * translate IOException from Jackson to HttpMessageNotReadableException
 * fix content negotiation issue when sorting selected media types by quality value
+* raise RestClientException instead of IllegalArgumentException for unknown status codes
 
 
 Changes in version 3.1.1 (2012-02-16)

org.springframework.web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.Charset;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.client.ClientHttpResponse;
@@ -28,10 +29,11 @@ import org.springframework.util.FileCopyUtils;
 /**
  * Default implementation of the {@link ResponseErrorHandler} interface.
  *
- * <p>This error handler checks for the status code on the {@link ClientHttpResponse}: any code with series
- * {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR} or
- * {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR} is considered to be an error.
- * This behavior can be changed by overriding the {@link #hasError(HttpStatus)} method.
+ * <p>This error handler checks for the status code on the {@link ClientHttpResponse}: any
+ * code with series {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR} or
+ * {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR} is considered to be an
+ * error. This behavior can be changed by overriding the {@link #hasError(HttpStatus)}
+ * method.
  *
  * @author Arjen Poutsma
  * @since 3.0
@@ -43,7 +45,18 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 * Delegates to {@link #hasError(HttpStatus)} with the response status code.
 	 */
 	public boolean hasError(ClientHttpResponse response) throws IOException {
-		return hasError(response.getStatusCode());
+		return hasError(getStatusCode(response));
+	}
+
+	private HttpStatus getStatusCode(ClientHttpResponse response) throws IOException {
+		HttpStatus statusCode;
+		try {
+			statusCode = response.getStatusCode();
+		}
+		catch (IllegalArgumentException ex) {
+			throw new RestClientException("Unknown status code [" + response.getRawStatusCode() + "]");
+		}
+		return statusCode;
 	}
 
 	/**
@@ -67,15 +80,16 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 * and a {@link RestClientException} in other cases.
 	 */
 	public void handleError(ClientHttpResponse response) throws IOException {
-		HttpStatus statusCode = response.getStatusCode();
-		MediaType contentType = response.getHeaders().getContentType();
+		HttpStatus statusCode = getStatusCode(response);
+		HttpHeaders headers = response.getHeaders();
+		MediaType contentType = headers.getContentType();
 		Charset charset = contentType != null ? contentType.getCharSet() : null;
 		byte[] body = getResponseBody(response);
 		switch (statusCode.series()) {
 			case CLIENT_ERROR:
-				throw new HttpClientErrorException(statusCode, response.getStatusText(), body, charset);
+				throw new HttpClientErrorException(statusCode, response.getStatusText(), headers, body, charset);
 			case SERVER_ERROR:
-				throw new HttpServerErrorException(statusCode, response.getStatusText(), body, charset);
+				throw new HttpServerErrorException(statusCode, response.getStatusText(), headers, body, charset);
 			default:
 				throw new RestClientException("Unknown status code [" + statusCode + "]");
 		}
@@ -83,15 +97,15 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 
 	private byte[] getResponseBody(ClientHttpResponse response) {
 		try {
-            InputStream responseBody = response.getBody();
-            if (responseBody != null) {
-                return FileCopyUtils.copyToByteArray(responseBody);
-            }
+			InputStream responseBody = response.getBody();
+			if (responseBody != null) {
+				return FileCopyUtils.copyToByteArray(responseBody);
+			}
 		}
 		catch (IOException ex) {
-            // ignore
+			// ignore
 		}
-        return new byte[0];
+		return new byte[0];
 	}
 
 }

org.springframework.web/src/main/java/org/springframework/web/client/HttpClientErrorException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -18,6 +18,7 @@ package org.springframework.web.client;
 
 import java.nio.charset.Charset;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 
 /**
@@ -29,8 +30,12 @@ import org.springframework.http.HttpStatus;
  */
 public class HttpClientErrorException extends HttpStatusCodeException {
 
+	private static final long serialVersionUID = 5177019431887513952L;
+
+
 	/**
-	 * Construct a new instance of {@code HttpClientErrorException} based on a {@link HttpStatus}.
+	 * Construct a new instance of {@code HttpClientErrorException} based on an
+	 * {@link HttpStatus}.
 	 * @param statusCode the status code
 	 */
 	public HttpClientErrorException(HttpStatus statusCode) {
@@ -38,7 +43,8 @@ public class HttpClientErrorException extends HttpStatusCodeException {
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpClientErrorException} based on a {@link HttpStatus} and status text.
+	 * Construct a new instance of {@code HttpClientErrorException} based on an
+	 * {@link HttpStatus} and status text.
 	 * @param statusCode the status code
 	 * @param statusText the status text
 	 */
@@ -47,18 +53,31 @@ public class HttpClientErrorException extends HttpStatusCodeException {
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpClientErrorException} based on a {@link HttpStatus}, status text, and
-	 * response body content.
-	 *
-	 * @param statusCode	  the status code
-	 * @param statusText	  the status text
-	 * @param responseBody	the response body content, may be {@code null}
+	 * Construct a new instance of {@code HttpClientErrorException} based on an
+	 * {@link HttpStatus}, status text, and response body content.
+	 * @param statusCode the status code
+	 * @param statusText the status text
+	 * @param responseBody the response body content, may be {@code null}
 	 * @param responseCharset the response body charset, may be {@code null}
 	 */
-	public HttpClientErrorException(HttpStatus statusCode,
-			String statusText,
-			byte[] responseBody,
-			Charset responseCharset) {
+	public HttpClientErrorException(HttpStatus statusCode, String statusText,
+			byte[] responseBody, Charset responseCharset) {
 		super(statusCode, statusText, responseBody, responseCharset);
 	}
+
+	/**
+	 * Construct a new instance of {@code HttpClientErrorException} based on an
+	 * {@link HttpStatus}, status text, and response body content.
+	 * @param statusCode the status code
+	 * @param statusText the status text
+	 * @param responseHeaders the response headers, may be {@code null}
+	 * @param responseBody the response body content, may be {@code null}
+	 * @param responseCharset the response body charset, may be {@code null}
+	 * @since 3.2
+	 */
+	public HttpClientErrorException(HttpStatus statusCode, String statusText,
+			HttpHeaders responseHeaders, byte[] responseBody, Charset responseCharset) {
+		super(statusCode, statusText, responseHeaders, responseBody, responseCharset);
+	}
+
 }

org.springframework.web/src/main/java/org/springframework/web/client/HttpServerErrorException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -18,6 +18,7 @@ package org.springframework.web.client;
 
 import java.nio.charset.Charset;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 
 /**
@@ -29,9 +30,12 @@ import org.springframework.http.HttpStatus;
  */
 public class HttpServerErrorException extends HttpStatusCodeException {
 
+	private static final long serialVersionUID = -2915754006618138282L;
+
+
 	/**
-	 * Construct a new instance of {@code HttpServerErrorException} based on a {@link HttpStatus}.
-	 *
+	 * Construct a new instance of {@code HttpServerErrorException} based on an
+	 * {@link HttpStatus}.
 	 * @param statusCode the status code
 	 */
 	public HttpServerErrorException(HttpStatus statusCode) {
@@ -39,8 +43,8 @@ public class HttpServerErrorException extends HttpStatusCodeException {
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpServerErrorException} based on a {@link HttpStatus} and status text.
-	 *
+	 * Construct a new instance of {@code HttpServerErrorException} based on an
+	 * {@link HttpStatus} and status text.
 	 * @param statusCode the status code
 	 * @param statusText the status text
 	 */
@@ -49,19 +53,31 @@ public class HttpServerErrorException extends HttpStatusCodeException {
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpServerErrorException} based on a {@link HttpStatus}, status text, and
-	 * response body content.
-	 *
+	 * Construct a new instance of {@code HttpServerErrorException} based on an
+	 * {@link HttpStatus}, status text, and response body content.
 	 * @param statusCode	  the status code
 	 * @param statusText	  the status text
 	 * @param responseBody	the response body content, may be {@code null}
 	 * @param responseCharset the response body charset, may be {@code null}
 	 * @since 3.0.5
 	 */
-	public HttpServerErrorException(HttpStatus statusCode,
-			String statusText,
-			byte[] responseBody,
-			Charset responseCharset) {
+	public HttpServerErrorException(HttpStatus statusCode, String statusText,
+			byte[] responseBody, Charset responseCharset) {
 		super(statusCode, statusText, responseBody, responseCharset);
 	}
+
+	/**
+	 * Construct a new instance of {@code HttpServerErrorException} based on a
+	 * {@link HttpStatus}, status text, and response body content.
+	 * @param statusCode the status code
+	 * @param statusText the status text
+	 * @param responseHeaders the response headers, may be {@code null}
+	 * @param responseBody the response body content, may be {@code null}
+	 * @param responseCharset the response body charset, may be {@code null}
+	 * @since 3.2
+	 */
+	public HttpServerErrorException(HttpStatus statusCode, String statusText,
+			HttpHeaders responseHeaders, byte[] responseBody, Charset responseCharset) {
+		super(statusCode, statusText, responseHeaders, responseBody, responseCharset);
+	}
 }

org.springframework.web/src/main/java/org/springframework/web/client/HttpStatusCodeException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -19,17 +19,21 @@ package org.springframework.web.client;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 
 /**
  * Abstract base class for exceptions based on an {@link HttpStatus}.
  *
  * @author Arjen Poutsma
+ * @author Chris Beams
  * @since 3.0
  */
 public abstract class HttpStatusCodeException extends RestClientException {
 
-	private static final Charset DEFAULT_CHARSET = Charset.forName("ISO-8859-1");
+	private static final long serialVersionUID = -5807494703720513267L;
+
+	private static final String DEFAULT_CHARSET = "ISO-8859-1";
 
 	private final HttpStatus statusCode;
 
@@ -37,34 +41,36 @@ public abstract class HttpStatusCodeException extends RestClientException {
 
 	private final byte[] responseBody;
 
-	private final Charset responseCharset;
+	private final HttpHeaders responseHeaders;
+
+	private final String responseCharset;
+
 
 	/**
-	 * Construct a new instance of {@code HttpStatusCodeException} based on a {@link HttpStatus}.
-	 *
+	 * Construct a new instance of {@code HttpStatusCodeException} based on an
+	 * {@link HttpStatus}.
 	 * @param statusCode the status code
 	 */
 	protected HttpStatusCodeException(HttpStatus statusCode) {
-		this(statusCode, statusCode.name(), null, null);
+		this(statusCode, statusCode.name(), null, null, null);
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpStatusCodeException} based on a {@link HttpStatus} and status text.
-	 *
+	 * Construct a new instance of {@code HttpStatusCodeException} based on an
+	 * {@link HttpStatus} and status text.
 	 * @param statusCode the status code
 	 * @param statusText the status text
 	 */
 	protected HttpStatusCodeException(HttpStatus statusCode, String statusText) {
-		this(statusCode, statusText, null, null);
+		this(statusCode, statusText, null, null, null);
 	}
 
 	/**
-	 * Construct a new instance of {@code HttpStatusCodeException} based on a {@link HttpStatus}, status text, and
-	 * response body content.
-	 *
-	 * @param statusCode	  the status code
-	 * @param statusText	  the status text
-	 * @param responseBody	the response body content, may be {@code null}
+	 * Construct a new instance of {@code HttpStatusCodeException} based on an
+	 * {@link HttpStatus}, status text, and response body content.
+	 * @param statusCode the status code
+	 * @param statusText the status text
+	 * @param responseBody the response body content, may be {@code null}
 	 * @param responseCharset the response body charset, may be {@code null}
 	 * @since 3.0.5
 	 */
@@ -72,29 +78,54 @@ public abstract class HttpStatusCodeException extends RestClientException {
 			String statusText,
 			byte[] responseBody,
 			Charset responseCharset) {
+		this(statusCode, statusText, null, responseBody, responseCharset);
+	}
+
+	/**
+	 * Construct a new instance of {@code HttpStatusCodeException} based on an
+	 * {@link HttpStatus}, status text, and response body content.
+	 * @param statusCode the status code
+	 * @param statusText the status text
+	 * @param responseHeaders the response headers, may be {@code null}
+	 * @param responseBody the response body content, may be {@code null}
+	 * @param responseCharset the response body charset, may be {@code null}
+	 * @since 3.2
+	 */
+	protected HttpStatusCodeException(HttpStatus statusCode, String statusText,
+			HttpHeaders responseHeaders, byte[] responseBody, Charset responseCharset) {
 		super(statusCode.value() + " " + statusText);
 		this.statusCode = statusCode;
 		this.statusText = statusText;
+		this.responseHeaders = responseHeaders;
 		this.responseBody = responseBody != null ? responseBody : new byte[0];
-		this.responseCharset = responseCharset != null ? responseCharset : DEFAULT_CHARSET;
+		this.responseCharset = responseCharset != null ? responseCharset.name() : DEFAULT_CHARSET;
 	}
 
+
 	/**
-	 * Returns the HTTP status code.
+	 * Return the HTTP status code.
 	 */
 	public HttpStatus getStatusCode() {
 		return this.statusCode;
 	}
 
 	/**
-	 * Returns the HTTP status text.
+	 * Return the HTTP status text.
 	 */
 	public String getStatusText() {
 		return this.statusText;
 	}
 
 	/**
-	 * Returns the response body as a byte array.
+	 * Return the HTTP response headers.
+	 * @since 3.2
+	 */
+	public HttpHeaders getResponseHeaders() {
+		return this.responseHeaders;
+	}
+
+	/**
+	 * Return the response body as a byte array.
 	 *
 	 * @since 3.0.5
 	 */
@@ -103,17 +134,17 @@ public abstract class HttpStatusCodeException extends RestClientException {
 	}
 
 	/**
-	 * Returns the response body as a string.
-	 *
+	 * Return the response body as a string.
 	 * @since 3.0.5
 	 */
 	public String getResponseBodyAsString() {
 		try {
-			return new String(responseBody, responseCharset.name());
+			return new String(responseBody, responseCharset);
 		}
 		catch (UnsupportedEncodingException ex) {
 			// should not occur
 			throw new InternalError(ex.getMessage());
 		}
 	}
+
 }

org.springframework.web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -19,19 +19,23 @@ package org.springframework.web.client;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
+import org.junit.Before;
+import org.junit.Test;
+
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.client.ClientHttpResponse;
 
-import org.junit.Before;
-import org.junit.Test;
-
 import static org.easymock.EasyMock.*;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
-/** @author Arjen Poutsma */
+import static org.junit.Assert.*;
+
+/**
+ * Unit tests for {@link DefaultResponseErrorHandler}.
+ *
+ * @author Arjen Poutsma
+ */
 public class DefaultResponseErrorHandlerTests {
 
 	private DefaultResponseErrorHandler handler;
@@ -64,7 +68,7 @@ public class DefaultResponseErrorHandlerTests {
 		verify(response);
 	}
 
-	@Test(expected = HttpClientErrorException.class)
+	@Test
 	public void handleError() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
@@ -76,7 +80,13 @@ public class DefaultResponseErrorHandlerTests {
 
 		replay(response);
 
-		handler.handleError(response);
+		try {
+			handler.handleError(response);
+			fail("expected HttpClientErrorException");
+		}
+		catch (HttpClientErrorException e) {
+			assertSame(headers, e.getResponseHeaders());
+		}
 
 		verify(response);
 	}
@@ -114,4 +124,16 @@ public class DefaultResponseErrorHandlerTests {
 
 		verify(response);
 	}
+
+	// SPR-9406
+
+	@Test(expected=RestClientException.class)
+	public void unknownStatusCode() throws Exception {
+		expect(response.getStatusCode()).andThrow(new IllegalArgumentException("No matching constant for 999"));
+		expect(response.getRawStatusCode()).andReturn(999);
+
+		replay(response);
+
+		handler.handleError(response);
+	}
 }