Consistent throwing of HttpMessageNotReadableException (5.0.x revision)

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

Issue: SPR-16995

Author: jhoeller

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

@@ -192,7 +192,9 @@ protected boolean canWrite(@Nullable MediaType mediaType) {
 	 * 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);
 	}
 
@@ -233,7 +235,7 @@ public HttpHeaders getHeaders() {
 	 * {@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

@@ -1,5 +1,5 @@
 /*
- * 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 @@ protected boolean supports(Class<?> clazz) {
 	}
 
 	@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 String getFilename() {
 			};
 		}
 		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

@@ -1,5 +1,5 @@
 /*
- * 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.
@@ -25,7 +25,7 @@
 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;
@@ -44,6 +44,7 @@
 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.*;
 
@@ -87,7 +88,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();
 
@@ -127,8 +128,8 @@ else if (ClassUtils.isPresent("com.google.protobuf.util.JsonFormat", getClass().
 			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);
@@ -179,6 +180,20 @@ else if (this.protobufFormatSupport != null) {
 		}
 	}
 
+	/**
+	 * 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) throws Exception {
+		Method method = methodCache.get(clazz);
+		if (method == null) {
+			method = clazz.getMethod("newBuilder");
+			methodCache.put(clazz, method);
+		}
+		return (Message.Builder) method.invoke(clazz);
+	}
+
+
 	@Override
 	protected boolean canWrite(@Nullable MediaType mediaType) {
 		return (super.canWrite(mediaType) ||
@@ -229,20 +244,6 @@ private void setProtoHeader(HttpOutputMessage response, Message message) {
 	}
 
 
-	/**
-	 * 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);
-	}
-
-
 	interface ProtobufFormatSupport {
 
 		MediaType[] supportedMediaTypes();
@@ -293,7 +294,7 @@ else if (contentType.isCompatibleWith(APPLICATION_XML)) {
 				this.xmlFormatter.merge(input, charset, extensionRegistry, builder);
 			}
 			else {
-				throw new IOException("com.google.protobuf.util does not support " + contentType + " format");
+				throw new IOException("protobuf-java-format does not support " + contentType + " format");
 			}
 		}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 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.
@@ -74,7 +74,7 @@ protected void customizeMarshaller(Marshaller marshaller) {
 	 * @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();
@@ -104,7 +104,7 @@ protected void customizeUnmarshaller(Unmarshaller unmarshaller) {
 	 * @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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 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.
@@ -29,7 +29,8 @@
 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}
@@ -57,12 +58,16 @@ protected AbstractXmlHttpMessageConverter() {
 
 
 	@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()));
 	}
 
@@ -84,20 +89,20 @@ protected void transform(Source source, Result result) throws TransformerExcepti
 	 * @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)}.
 	 * @param t the object to write to the output message
 	 * @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

@@ -1,11 +1,11 @@
 /*
- * 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.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *      http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -159,21 +159,21 @@ else if (elementClass.isAnnotationPresent(XmlType.class)) {
 				}
 				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

@@ -1,5 +1,5 @@
 /*
- * 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 @@ protected Object readFromSource(Class<?> clazz, HttpHeaders headers, Source sour
 		}
 		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 @@ protected void writeToResult(Object o, HttpHeaders headers, Result result) throw
 			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

@@ -1,5 +1,5 @@
 /*
- * 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.
@@ -50,7 +50,6 @@
 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;
@@ -159,7 +158,7 @@ else if (StreamSource.class == clazz || Source.class == clazz) {
 			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

@@ -1,5 +1,5 @@
 /*
- * 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 void shouldReadInputStreamResource() throws IOException {
 	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);