Consistent throwing of HttpMessageNotReadableException vs IOException

Includes specific fine-tuning of ProtobufHttpMessageConverter and JAXB2 based message converters, as well as revised javadoc for abstract base classes.

Issue: SPR-16995

spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java

@@ -193,7 +193,9 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	 * Future implementations might add some default behavior, however.
 	 */
 	@Override
-	public final T read(Class<? extends T> clazz, HttpInputMessage inputMessage) throws IOException {
+	public final T read(Class<? extends T> clazz, HttpInputMessage inputMessage)
+			throws IOException, HttpMessageNotReadableException {
+
 		return readInternal(clazz, inputMessage);
 	}
 
@@ -234,7 +236,7 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	 * {@link #getContentLength}, and sets the corresponding headers.
 	 * @since 4.2
 	 */
-	protected void addDefaultHeaders(HttpHeaders headers, T t, @Nullable MediaType contentType) throws IOException{
+	protected void addDefaultHeaders(HttpHeaders headers, T t, @Nullable MediaType contentType) throws IOException {
 		if (headers.getContentType() == null) {
 			MediaType contentTypeToUse = contentType;
 			if (contentType == null || contentType.isWildcardType() || contentType.isWildcardSubtype()) {

spring-web/src/main/java/org/springframework/http/converter/ObjectToStringHttpMessageConverter.java

 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -106,11 +106,13 @@ public class ObjectToStringHttpMessageConverter extends AbstractHttpMessageConve
 	}
 
 	@Override
-	protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage) throws IOException {
+	protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage)
+			throws IOException, HttpMessageNotReadableException {
+
 		String value = this.stringHttpMessageConverter.readInternal(String.class, inputMessage);
 		Object result = this.conversionService.convert(value, clazz);
 		if (result == null) {
-			throw new HttpMessageConversionException(
+			throw new HttpMessageNotReadableException(
 					"Unexpected null conversion result for '" + value + "' to " + clazz);
 		}
 		return result;

spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java

@@ -97,7 +97,7 @@ public class ResourceHttpMessageConverter extends AbstractHttpMessageConverter<R
 			};
 		}
 		else {
-			throw new IllegalStateException("Unsupported resource class: " + clazz);
+			throw new HttpMessageNotReadableException("Unsupported resource class: " + clazz);
 		}
 	}
 

spring-web/src/main/java/org/springframework/http/converter/protobuf/ProtobufHttpMessageConverter.java

@@ -25,7 +25,7 @@ import java.lang.reflect.Method;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Map;
 
 import com.google.protobuf.CodedOutputStream;
 import com.google.protobuf.ExtensionRegistry;
@@ -39,11 +39,13 @@ import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.AbstractHttpMessageConverter;
+import org.springframework.http.converter.HttpMessageConversionException;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.http.converter.HttpMessageNotWritableException;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.ConcurrentReferenceHashMap;
 
 import static org.springframework.http.MediaType.APPLICATION_JSON;
 import static org.springframework.http.MediaType.APPLICATION_XML;
@@ -51,8 +53,9 @@ import static org.springframework.http.MediaType.TEXT_HTML;
 import static org.springframework.http.MediaType.TEXT_PLAIN;
 
 /**
- * An {@code HttpMessageConverter} that reads and writes {@link com.google.protobuf.Message com.google.protobuf.Messages}
- * using <a href="https://developers.google.com/protocol-buffers/">Google Protocol Buffers</a>.
+ * An {@code HttpMessageConverter} that reads and writes
+ * {@link com.google.protobuf.Message com.google.protobuf.Messages} using
+ * <a href="https://developers.google.com/protocol-buffers/">Google Protocol Buffers</a>.
  *
  * <p>To generate {@code Message} Java classes, you need to install the {@code protoc} binary.
  *
@@ -102,7 +105,7 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 	public static final String X_PROTOBUF_MESSAGE_HEADER = "X-Protobuf-Message";
 
 
-	private static final ConcurrentHashMap<Class<?>, Method> methodCache = new ConcurrentHashMap<>();
+	private static final Map<Class<?>, Method> methodCache = new ConcurrentReferenceHashMap<>();
 
 	private final ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
 
@@ -142,8 +145,8 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 			this.protobufFormatSupport = null;
 		}
 
-		setSupportedMediaTypes(Arrays.asList((this.protobufFormatSupport != null ?
-				this.protobufFormatSupport.supportedMediaTypes() : new MediaType[] {PROTOBUF, TEXT_PLAIN})));
+		setSupportedMediaTypes(Arrays.asList(this.protobufFormatSupport != null ?
+				this.protobufFormatSupport.supportedMediaTypes() : new MediaType[] {PROTOBUF, TEXT_PLAIN}));
 
 		if (registryInitializer != null) {
 			registryInitializer.initializeExtensionRegistry(this.extensionRegistry);
@@ -174,26 +177,41 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 			charset = DEFAULT_CHARSET;
 		}
 
+		Message.Builder builder = getMessageBuilder(clazz);
+		if (PROTOBUF.isCompatibleWith(contentType)) {
+			builder.mergeFrom(inputMessage.getBody(), this.extensionRegistry);
+		}
+		else if (TEXT_PLAIN.isCompatibleWith(contentType)) {
+			InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
+			TextFormat.merge(reader, this.extensionRegistry, builder);
+		}
+		else if (this.protobufFormatSupport != null) {
+			this.protobufFormatSupport.merge(
+					inputMessage.getBody(), charset, contentType, this.extensionRegistry, builder);
+		}
+		return builder.build();
+	}
+
+	/**
+	 * Create a new {@code Message.Builder} instance for the given class.
+	 * <p>This method uses a ConcurrentReferenceHashMap for caching method lookups.
+	 */
+	private Message.Builder getMessageBuilder(Class<? extends Message> clazz) {
 		try {
-			Message.Builder builder = getMessageBuilder(clazz);
-			if (PROTOBUF.isCompatibleWith(contentType)) {
-				builder.mergeFrom(inputMessage.getBody(), this.extensionRegistry);
+			Method method = methodCache.get(clazz);
+			if (method == null) {
+				method = clazz.getMethod("newBuilder");
+				methodCache.put(clazz, method);
 			}
-			else if (TEXT_PLAIN.isCompatibleWith(contentType)) {
-				InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
-				TextFormat.merge(reader, this.extensionRegistry, builder);
-			}
-			else if (this.protobufFormatSupport != null) {
-				this.protobufFormatSupport.merge(inputMessage.getBody(), charset, contentType,
-						this.extensionRegistry, builder);
-			}
-			return builder.build();
+			return (Message.Builder) method.invoke(clazz);
 		}
 		catch (Exception ex) {
-			throw new HttpMessageNotReadableException("Could not read Protobuf message: " + ex.getMessage(), ex);
+			throw new HttpMessageConversionException(
+					"Invalid Protobuf Message type: no invocable newBuilder() method on " + clazz, ex);
 		}
 	}
 
+
 	@Override
 	protected boolean canWrite(@Nullable MediaType mediaType) {
 		return (super.canWrite(mediaType) ||
@@ -244,20 +262,6 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 	}
 
 
-	/**
-	 * Create a new {@code Message.Builder} instance for the given class.
-	 * <p>This method uses a ConcurrentHashMap for caching method lookups.
-	 */
-	private static Message.Builder getMessageBuilder(Class<? extends Message> clazz) throws Exception {
-		Method method = methodCache.get(clazz);
-		if (method == null) {
-			method = clazz.getMethod("newBuilder");
-			methodCache.put(clazz, method);
-		}
-		return (Message.Builder) method.invoke(clazz);
-	}
-
-
 	/**
 	 * Protobuf format support.
 	 */
@@ -268,10 +272,11 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 		boolean supportsWriteOnly(@Nullable MediaType mediaType);
 
 		void merge(InputStream input, Charset charset, MediaType contentType,
-				ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException;
+				ExtensionRegistry extensionRegistry, Message.Builder builder)
+				throws IOException, HttpMessageNotReadableException;
 
 		void print(Message message, OutputStream output, MediaType contentType, Charset charset)
-				throws IOException;
+				throws IOException, HttpMessageNotWritableException;
 	}
 
 	/**
@@ -305,7 +310,8 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 
 		@Override
 		public void merge(InputStream input, Charset charset, MediaType contentType,
-				ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException {
+				ExtensionRegistry extensionRegistry, Message.Builder builder)
+				throws IOException, HttpMessageNotReadableException {
 
 			if (contentType.isCompatibleWith(APPLICATION_JSON)) {
 				this.jsonFormatter.merge(input, charset, extensionRegistry, builder);
@@ -314,13 +320,14 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 				this.xmlFormatter.merge(input, charset, extensionRegistry, builder);
 			}
 			else {
-				throw new IOException("com.google.protobuf.util does not support " + contentType + " format");
+				throw new HttpMessageNotReadableException(
+						"protobuf-java-format does not support parsing " + contentType);
 			}
 		}
 
 		@Override
 		public void print(Message message, OutputStream output, MediaType contentType, Charset charset)
-				throws IOException {
+				throws IOException, HttpMessageNotWritableException {
 
 			if (contentType.isCompatibleWith(APPLICATION_JSON)) {
 				this.jsonFormatter.print(message, output, charset);
@@ -332,7 +339,8 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 				this.htmlFormatter.print(message, output, charset);
 			}
 			else {
-				throw new IOException("protobuf-java-format does not support " + contentType + " format");
+				throw new HttpMessageNotWritableException(
+						"protobuf-java-format does not support printing " + contentType);
 			}
 		}
 	}
@@ -365,20 +373,22 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 
 		@Override
 		public void merge(InputStream input, Charset charset, MediaType contentType,
-				ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException {
+				ExtensionRegistry extensionRegistry, Message.Builder builder)
+				throws IOException, HttpMessageNotReadableException {
 
 			if (contentType.isCompatibleWith(APPLICATION_JSON)) {
 				InputStreamReader reader = new InputStreamReader(input, charset);
 				this.parser.merge(reader, builder);
 			}
 			else {
-				throw new IOException("protobuf-java-util does not support " + contentType + " format");
+				throw new HttpMessageNotReadableException(
+						"protobuf-java-util does not support parsing " + contentType);
 			}
 		}
 
 		@Override
 		public void print(Message message, OutputStream output, MediaType contentType, Charset charset)
-				throws IOException {
+				throws IOException, HttpMessageNotWritableException {
 
 			if (contentType.isCompatibleWith(APPLICATION_JSON)) {
 				OutputStreamWriter writer = new OutputStreamWriter(output, charset);
@@ -386,7 +396,8 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
 				writer.flush();
 			}
 			else {
-				throw new IOException("protobuf-java-util does not support " + contentType + " format");
+				throw new HttpMessageNotWritableException(
+						"protobuf-java-util does not support printing " + contentType);
 			}
 		}
 	}

spring-web/src/main/java/org/springframework/http/converter/xml/AbstractJaxb2HttpMessageConverter.java

@@ -75,7 +75,7 @@ public abstract class AbstractJaxb2HttpMessageConverter<T> extends AbstractXmlHt
 	 * @return the {@code Unmarshaller}
 	 * @throws HttpMessageConversionException in case of JAXB errors
 	 */
-	protected final Unmarshaller createUnmarshaller(Class<?> clazz) throws JAXBException {
+	protected final Unmarshaller createUnmarshaller(Class<?> clazz) {
 		try {
 			JAXBContext jaxbContext = getJaxbContext(clazz);
 			Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
@@ -105,7 +105,7 @@ public abstract class AbstractJaxb2HttpMessageConverter<T> extends AbstractXmlHt
 	 * @throws HttpMessageConversionException in case of JAXB errors
 	 */
 	protected final JAXBContext getJaxbContext(Class<?> clazz) {
-		Assert.notNull(clazz, "'clazz' must not be null");
+		Assert.notNull(clazz, "Class must not be null");
 		JAXBContext jaxbContext = this.jaxbContexts.get(clazz);
 		if (jaxbContext == null) {
 			try {

spring-web/src/main/java/org/springframework/http/converter/xml/AbstractXmlHttpMessageConverter.java

@@ -29,7 +29,8 @@ import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.AbstractHttpMessageConverter;
-import org.springframework.http.converter.HttpMessageConversionException;
+import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.springframework.http.converter.HttpMessageNotWritableException;
 
 /**
  * Abstract base class for {@link org.springframework.http.converter.HttpMessageConverter HttpMessageConverters}
@@ -58,12 +59,16 @@ public abstract class AbstractXmlHttpMessageConverter<T> extends AbstractHttpMes
 
 
 	@Override
-	public final T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage) throws IOException {
+	public final T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage)
+			throws IOException, HttpMessageNotReadableException {
+
 		return readFromSource(clazz, inputMessage.getHeaders(), new StreamSource(inputMessage.getBody()));
 	}
 
 	@Override
-	protected final void writeInternal(T t, HttpOutputMessage outputMessage) throws IOException {
+	protected final void writeInternal(T t, HttpOutputMessage outputMessage)
+			throws IOException, HttpMessageNotWritableException {
+
 		writeToResult(t, outputMessage.getHeaders(), new StreamResult(outputMessage.getBody()));
 	}
 
@@ -85,10 +90,10 @@ public abstract class AbstractXmlHttpMessageConverter<T> extends AbstractHttpMes
 	 * @param source the HTTP input body
 	 * @return the converted object
 	 * @throws IOException in case of I/O errors
-	 * @throws org.springframework.http.converter.HttpMessageConversionException in case of conversion errors
+	 * @throws HttpMessageNotReadableException in case of conversion errors
 	 */
 	protected abstract T readFromSource(Class<? extends T> clazz, HttpHeaders headers, Source source)
-			throws IOException;
+			throws IOException, HttpMessageNotReadableException;
 
 	/**
 	 * Abstract template method called from {@link #writeInternal(Object, HttpOutputMessage)}.
@@ -96,9 +101,9 @@ public abstract class AbstractXmlHttpMessageConverter<T> extends AbstractHttpMes
 	 * @param headers the HTTP output headers
 	 * @param result the HTTP output body
 	 * @throws IOException in case of I/O errors
-	 * @throws HttpMessageConversionException in case of conversion errors
+	 * @throws HttpMessageNotWritableException in case of conversion errors
 	 */
 	protected abstract void writeToResult(T t, HttpHeaders headers, Result result)
-			throws IOException;
+			throws IOException, HttpMessageNotWritableException;
 
 }

spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java

@@ -160,21 +160,21 @@ public class Jaxb2CollectionHttpMessageConverter<T extends Collection>
 				}
 				else {
 					// should not happen, since we check in canRead(Type)
-					throw new HttpMessageConversionException("Could not unmarshal to [" + elementClass + "]");
+					throw new HttpMessageNotReadableException("Cannot unmarshal to [" + elementClass + "]");
 				}
 				event = moveToNextElement(streamReader);
 			}
 			return result;
 		}
+		catch (XMLStreamException ex) {
+			throw new HttpMessageNotReadableException("Failed to read XML stream: " + ex.getMessage(), ex);
+		}
 		catch (UnmarshalException ex) {
 			throw new HttpMessageNotReadableException(
 					"Could not unmarshal to [" + elementClass + "]: " + ex.getMessage(), ex);
 		}
 		catch (JAXBException ex) {
-			throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
-		}
-		catch (XMLStreamException ex) {
-			throw new HttpMessageConversionException(ex.getMessage(), ex);
+			throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
 		}
 	}
 

spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java

 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -145,10 +145,9 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa
 		}
 		catch (UnmarshalException ex) {
 			throw new HttpMessageNotReadableException("Could not unmarshal to [" + clazz + "]: " + ex.getMessage(), ex);
-
 		}
 		catch (JAXBException ex) {
-			throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
+			throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
 		}
 	}
 
@@ -189,7 +188,7 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa
 			throw new HttpMessageNotWritableException("Could not marshal [" + o + "]: " + ex.getMessage(), ex);
 		}
 		catch (JAXBException ex) {
-			throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
+			throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
 		}
 	}
 

spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java

@@ -50,7 +50,6 @@ import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.AbstractHttpMessageConverter;
-import org.springframework.http.converter.HttpMessageConversionException;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.http.converter.HttpMessageNotWritableException;
 import org.springframework.lang.Nullable;
@@ -160,7 +159,7 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 			return (T) readStreamSource(body);
 		}
 		else {
-			throw new HttpMessageConversionException("Could not read class [" + clazz +
+			throw new HttpMessageNotReadableException("Could not read class [" + clazz +
 					"]. Only DOMSource, SAXSource, StAXSource, and StreamSource are supported.");
 		}
 	}

spring-web/src/test/java/org/springframework/http/converter/ResourceHttpMessageConverterTests.java

 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -95,7 +95,7 @@ public class ResourceHttpMessageConverterTests {
 	public void shouldNotReadInputStreamResource() throws IOException {
 		ResourceHttpMessageConverter noStreamConverter = new ResourceHttpMessageConverter(false);
 		try (InputStream body = getClass().getResourceAsStream("logo.jpg") ) {
-			this.thrown.expect(IllegalStateException.class);
+			this.thrown.expect(HttpMessageNotReadableException.class);
 			MockHttpInputMessage inputMessage = new MockHttpInputMessage(body);
 			inputMessage.getHeaders().setContentType(MediaType.IMAGE_JPEG);
 			noStreamConverter.read(InputStreamResource.class, inputMessage);