From d08b72a75a4671f456f240eff9ca39885be7d475 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 2 Jul 2018 22:37:29 +0200 Subject: [PATCH] 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 --- .../AbstractHttpMessageConverter.java | 6 +- .../ObjectToStringHttpMessageConverter.java | 8 +- .../ResourceHttpMessageConverter.java | 2 +- .../ProtobufHttpMessageConverter.java | 97 +++++++++++-------- .../AbstractJaxb2HttpMessageConverter.java | 4 +- .../xml/AbstractXmlHttpMessageConverter.java | 19 ++-- .../Jaxb2CollectionHttpMessageConverter.java | 10 +- .../Jaxb2RootElementHttpMessageConverter.java | 7 +- .../xml/SourceHttpMessageConverter.java | 3 +- .../ResourceHttpMessageConverterTests.java | 4 +- 10 files changed, 89 insertions(+), 71 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java index 59757b30dd..f9ae204336 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java @@ -193,7 +193,9 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv * Future implementations might add some default behavior, however. */ @Override - public final T read(Class clazz, HttpInputMessage inputMessage) throws IOException { + public final T read(Class clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + return readInternal(clazz, inputMessage); } @@ -234,7 +236,7 @@ public abstract class AbstractHttpMessageConverter 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()) { diff --git a/spring-web/src/main/java/org/springframework/http/converter/ObjectToStringHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/ObjectToStringHttpMessageConverter.java index 4ed450ec6d..039185fad4 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/ObjectToStringHttpMessageConverter.java +++ b/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 @@ 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; diff --git a/spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java index 0f84846454..8dc4dc9580 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java @@ -97,7 +97,7 @@ public class ResourceHttpMessageConverter extends AbstractHttpMessageConverterGoogle Protocol Buffers. + * An {@code HttpMessageConverter} that reads and writes + * {@link com.google.protobuf.Message com.google.protobuf.Messages} using + * Google Protocol Buffers. * *

To generate {@code Message} Java classes, you need to install the {@code protoc} binary. * @@ -102,7 +105,7 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter, Method> methodCache = new ConcurrentHashMap<>(); + private static final Map, Method> methodCache = new ConcurrentReferenceHashMap<>(); private final ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance(); @@ -142,8 +145,8 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverterThis method uses a ConcurrentReferenceHashMap for caching method lookups. + */ + private Message.Builder getMessageBuilder(Class 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 AbstractHttpMessageConverterThis method uses a ConcurrentHashMap for caching method lookups. - */ - private static Message.Builder getMessageBuilder(Class 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 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 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 { diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/AbstractXmlHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/AbstractXmlHttpMessageConverter.java index d5e611fd2f..720a966068 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/AbstractXmlHttpMessageConverter.java +++ b/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 extends AbstractHttpMes @Override - public final T readInternal(Class clazz, HttpInputMessage inputMessage) throws IOException { + public final T readInternal(Class 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 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 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 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; } diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java index 9b947901c8..cdc341c8ea 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java @@ -160,21 +160,21 @@ public class Jaxb2CollectionHttpMessageConverter } 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); } } diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java index 7bc08150c1..a6e6708393 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java +++ b/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 @@ 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); } } diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java index 954e07074a..da358a8535 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java +++ b/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 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."); } } diff --git a/spring-web/src/test/java/org/springframework/http/converter/ResourceHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/ResourceHttpMessageConverterTests.java index 10dcedf34e..00ce3f04b9 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/ResourceHttpMessageConverterTests.java +++ b/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 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);