Skip to content

Commit d08b72a

Browse files
jhoellerjhoeller
committed
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
1 parent 779cf8d commit d08b72a

10 files changed

+89
-71
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,9 @@ protected boolean canWrite(@Nullable MediaType mediaType) {
193193
* Future implementations might add some default behavior, however.
194194
*/
195195
@Override
196-
public final T read(Class<? extends T> clazz, HttpInputMessage inputMessage) throws IOException {
196+
public final T read(Class<? extends T> clazz, HttpInputMessage inputMessage)
197+
throws IOException, HttpMessageNotReadableException {
198+
197199
return readInternal(clazz, inputMessage);
198200
}
199201

@@ -234,7 +236,7 @@ public HttpHeaders getHeaders() {
234236
* {@link #getContentLength}, and sets the corresponding headers.
235237
* @since 4.2
236238
*/
237-
protected void addDefaultHeaders(HttpHeaders headers, T t, @Nullable MediaType contentType) throws IOException{
239+
protected void addDefaultHeaders(HttpHeaders headers, T t, @Nullable MediaType contentType) throws IOException {
238240
if (headers.getContentType() == null) {
239241
MediaType contentTypeToUse = contentType;
240242
if (contentType == null || contentType.isWildcardType() || contentType.isWildcardSubtype()) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -106,11 +106,13 @@ protected boolean supports(Class<?> clazz) {
106106
}
107107

108108
@Override
109-
protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage) throws IOException {
109+
protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage)
110+
throws IOException, HttpMessageNotReadableException {
111+
110112
String value = this.stringHttpMessageConverter.readInternal(String.class, inputMessage);
111113
Object result = this.conversionService.convert(value, clazz);
112114
if (result == null) {
113-
throw new HttpMessageConversionException(
115+
throw new HttpMessageNotReadableException(
114116
"Unexpected null conversion result for '" + value + "' to " + clazz);
115117
}
116118
return result;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public String getFilename() {
9797
};
9898
}
9999
else {
100-
throw new IllegalStateException("Unsupported resource class: " + clazz);
100+
throw new HttpMessageNotReadableException("Unsupported resource class: " + clazz);
101101
}
102102
}
103103

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

Lines changed: 54 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.nio.charset.Charset;
2626
import java.nio.charset.StandardCharsets;
2727
import java.util.Arrays;
28-
import java.util.concurrent.ConcurrentHashMap;
28+
import java.util.Map;
2929

3030
import com.google.protobuf.CodedOutputStream;
3131
import com.google.protobuf.ExtensionRegistry;
@@ -39,20 +39,23 @@
3939
import org.springframework.http.HttpOutputMessage;
4040
import org.springframework.http.MediaType;
4141
import org.springframework.http.converter.AbstractHttpMessageConverter;
42+
import org.springframework.http.converter.HttpMessageConversionException;
4243
import org.springframework.http.converter.HttpMessageNotReadableException;
4344
import org.springframework.http.converter.HttpMessageNotWritableException;
4445
import org.springframework.lang.Nullable;
4546
import org.springframework.util.Assert;
4647
import org.springframework.util.ClassUtils;
48+
import org.springframework.util.ConcurrentReferenceHashMap;
4749

4850
import static org.springframework.http.MediaType.APPLICATION_JSON;
4951
import static org.springframework.http.MediaType.APPLICATION_XML;
5052
import static org.springframework.http.MediaType.TEXT_HTML;
5153
import static org.springframework.http.MediaType.TEXT_PLAIN;
5254

5355
/**
54-
* An {@code HttpMessageConverter} that reads and writes {@link com.google.protobuf.Message com.google.protobuf.Messages}
55-
* using <a href="https://developers.google.com/protocol-buffers/">Google Protocol Buffers</a>.
56+
* An {@code HttpMessageConverter} that reads and writes
57+
* {@link com.google.protobuf.Message com.google.protobuf.Messages} using
58+
* <a href="https://developers.google.com/protocol-buffers/">Google Protocol Buffers</a>.
5659
*
5760
* <p>To generate {@code Message} Java classes, you need to install the {@code protoc} binary.
5861
*
@@ -102,7 +105,7 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
102105
public static final String X_PROTOBUF_MESSAGE_HEADER = "X-Protobuf-Message";
103106

104107

105-
private static final ConcurrentHashMap<Class<?>, Method> methodCache = new ConcurrentHashMap<>();
108+
private static final Map<Class<?>, Method> methodCache = new ConcurrentReferenceHashMap<>();
106109

107110
private final ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
108111

@@ -142,8 +145,8 @@ else if (ClassUtils.isPresent("com.google.protobuf.util.JsonFormat", getClass().
142145
this.protobufFormatSupport = null;
143146
}
144147

145-
setSupportedMediaTypes(Arrays.asList((this.protobufFormatSupport != null ?
146-
this.protobufFormatSupport.supportedMediaTypes() : new MediaType[] {PROTOBUF, TEXT_PLAIN})));
148+
setSupportedMediaTypes(Arrays.asList(this.protobufFormatSupport != null ?
149+
this.protobufFormatSupport.supportedMediaTypes() : new MediaType[] {PROTOBUF, TEXT_PLAIN}));
147150

148151
if (registryInitializer != null) {
149152
registryInitializer.initializeExtensionRegistry(this.extensionRegistry);
@@ -174,26 +177,41 @@ protected Message readInternal(Class<? extends Message> clazz, HttpInputMessage
174177
charset = DEFAULT_CHARSET;
175178
}
176179

180+
Message.Builder builder = getMessageBuilder(clazz);
181+
if (PROTOBUF.isCompatibleWith(contentType)) {
182+
builder.mergeFrom(inputMessage.getBody(), this.extensionRegistry);
183+
}
184+
else if (TEXT_PLAIN.isCompatibleWith(contentType)) {
185+
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
186+
TextFormat.merge(reader, this.extensionRegistry, builder);
187+
}
188+
else if (this.protobufFormatSupport != null) {
189+
this.protobufFormatSupport.merge(
190+
inputMessage.getBody(), charset, contentType, this.extensionRegistry, builder);
191+
}
192+
return builder.build();
193+
}
194+
195+
/**
196+
* Create a new {@code Message.Builder} instance for the given class.
197+
* <p>This method uses a ConcurrentReferenceHashMap for caching method lookups.
198+
*/
199+
private Message.Builder getMessageBuilder(Class<? extends Message> clazz) {
177200
try {
178-
Message.Builder builder = getMessageBuilder(clazz);
179-
if (PROTOBUF.isCompatibleWith(contentType)) {
180-
builder.mergeFrom(inputMessage.getBody(), this.extensionRegistry);
201+
Method method = methodCache.get(clazz);
202+
if (method == null) {
203+
method = clazz.getMethod("newBuilder");
204+
methodCache.put(clazz, method);
181205
}
182-
else if (TEXT_PLAIN.isCompatibleWith(contentType)) {
183-
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
184-
TextFormat.merge(reader, this.extensionRegistry, builder);
185-
}
186-
else if (this.protobufFormatSupport != null) {
187-
this.protobufFormatSupport.merge(inputMessage.getBody(), charset, contentType,
188-
this.extensionRegistry, builder);
189-
}
190-
return builder.build();
206+
return (Message.Builder) method.invoke(clazz);
191207
}
192208
catch (Exception ex) {
193-
throw new HttpMessageNotReadableException("Could not read Protobuf message: " + ex.getMessage(), ex);
209+
throw new HttpMessageConversionException(
210+
"Invalid Protobuf Message type: no invocable newBuilder() method on " + clazz, ex);
194211
}
195212
}
196213

214+
197215
@Override
198216
protected boolean canWrite(@Nullable MediaType mediaType) {
199217
return (super.canWrite(mediaType) ||
@@ -244,20 +262,6 @@ private void setProtoHeader(HttpOutputMessage response, Message message) {
244262
}
245263

246264

247-
/**
248-
* Create a new {@code Message.Builder} instance for the given class.
249-
* <p>This method uses a ConcurrentHashMap for caching method lookups.
250-
*/
251-
private static Message.Builder getMessageBuilder(Class<? extends Message> clazz) throws Exception {
252-
Method method = methodCache.get(clazz);
253-
if (method == null) {
254-
method = clazz.getMethod("newBuilder");
255-
methodCache.put(clazz, method);
256-
}
257-
return (Message.Builder) method.invoke(clazz);
258-
}
259-
260-
261265
/**
262266
* Protobuf format support.
263267
*/
@@ -268,10 +272,11 @@ interface ProtobufFormatSupport {
268272
boolean supportsWriteOnly(@Nullable MediaType mediaType);
269273

270274
void merge(InputStream input, Charset charset, MediaType contentType,
271-
ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException;
275+
ExtensionRegistry extensionRegistry, Message.Builder builder)
276+
throws IOException, HttpMessageNotReadableException;
272277

273278
void print(Message message, OutputStream output, MediaType contentType, Charset charset)
274-
throws IOException;
279+
throws IOException, HttpMessageNotWritableException;
275280
}
276281

277282
/**
@@ -305,7 +310,8 @@ public boolean supportsWriteOnly(@Nullable MediaType mediaType) {
305310

306311
@Override
307312
public void merge(InputStream input, Charset charset, MediaType contentType,
308-
ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException {
313+
ExtensionRegistry extensionRegistry, Message.Builder builder)
314+
throws IOException, HttpMessageNotReadableException {
309315

310316
if (contentType.isCompatibleWith(APPLICATION_JSON)) {
311317
this.jsonFormatter.merge(input, charset, extensionRegistry, builder);
@@ -314,13 +320,14 @@ else if (contentType.isCompatibleWith(APPLICATION_XML)) {
314320
this.xmlFormatter.merge(input, charset, extensionRegistry, builder);
315321
}
316322
else {
317-
throw new IOException("com.google.protobuf.util does not support " + contentType + " format");
323+
throw new HttpMessageNotReadableException(
324+
"protobuf-java-format does not support parsing " + contentType);
318325
}
319326
}
320327

321328
@Override
322329
public void print(Message message, OutputStream output, MediaType contentType, Charset charset)
323-
throws IOException {
330+
throws IOException, HttpMessageNotWritableException {
324331

325332
if (contentType.isCompatibleWith(APPLICATION_JSON)) {
326333
this.jsonFormatter.print(message, output, charset);
@@ -332,7 +339,8 @@ else if (contentType.isCompatibleWith(TEXT_HTML)) {
332339
this.htmlFormatter.print(message, output, charset);
333340
}
334341
else {
335-
throw new IOException("protobuf-java-format does not support " + contentType + " format");
342+
throw new HttpMessageNotWritableException(
343+
"protobuf-java-format does not support printing " + contentType);
336344
}
337345
}
338346
}
@@ -365,28 +373,31 @@ public boolean supportsWriteOnly(@Nullable MediaType mediaType) {
365373

366374
@Override
367375
public void merge(InputStream input, Charset charset, MediaType contentType,
368-
ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException {
376+
ExtensionRegistry extensionRegistry, Message.Builder builder)
377+
throws IOException, HttpMessageNotReadableException {
369378

370379
if (contentType.isCompatibleWith(APPLICATION_JSON)) {
371380
InputStreamReader reader = new InputStreamReader(input, charset);
372381
this.parser.merge(reader, builder);
373382
}
374383
else {
375-
throw new IOException("protobuf-java-util does not support " + contentType + " format");
384+
throw new HttpMessageNotReadableException(
385+
"protobuf-java-util does not support parsing " + contentType);
376386
}
377387
}
378388

379389
@Override
380390
public void print(Message message, OutputStream output, MediaType contentType, Charset charset)
381-
throws IOException {
391+
throws IOException, HttpMessageNotWritableException {
382392

383393
if (contentType.isCompatibleWith(APPLICATION_JSON)) {
384394
OutputStreamWriter writer = new OutputStreamWriter(output, charset);
385395
this.printer.appendTo(message, writer);
386396
writer.flush();
387397
}
388398
else {
389-
throw new IOException("protobuf-java-util does not support " + contentType + " format");
399+
throw new HttpMessageNotWritableException(
400+
"protobuf-java-util does not support printing " + contentType);
390401
}
391402
}
392403
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected void customizeMarshaller(Marshaller marshaller) {
7575
* @return the {@code Unmarshaller}
7676
* @throws HttpMessageConversionException in case of JAXB errors
7777
*/
78-
protected final Unmarshaller createUnmarshaller(Class<?> clazz) throws JAXBException {
78+
protected final Unmarshaller createUnmarshaller(Class<?> clazz) {
7979
try {
8080
JAXBContext jaxbContext = getJaxbContext(clazz);
8181
Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
@@ -105,7 +105,7 @@ protected void customizeUnmarshaller(Unmarshaller unmarshaller) {
105105
* @throws HttpMessageConversionException in case of JAXB errors
106106
*/
107107
protected final JAXBContext getJaxbContext(Class<?> clazz) {
108-
Assert.notNull(clazz, "'clazz' must not be null");
108+
Assert.notNull(clazz, "Class must not be null");
109109
JAXBContext jaxbContext = this.jaxbContexts.get(clazz);
110110
if (jaxbContext == null) {
111111
try {

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
import org.springframework.http.HttpOutputMessage;
3030
import org.springframework.http.MediaType;
3131
import org.springframework.http.converter.AbstractHttpMessageConverter;
32-
import org.springframework.http.converter.HttpMessageConversionException;
32+
import org.springframework.http.converter.HttpMessageNotReadableException;
33+
import org.springframework.http.converter.HttpMessageNotWritableException;
3334

3435
/**
3536
* Abstract base class for {@link org.springframework.http.converter.HttpMessageConverter HttpMessageConverters}
@@ -58,12 +59,16 @@ protected AbstractXmlHttpMessageConverter() {
5859

5960

6061
@Override
61-
public final T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage) throws IOException {
62+
public final T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage)
63+
throws IOException, HttpMessageNotReadableException {
64+
6265
return readFromSource(clazz, inputMessage.getHeaders(), new StreamSource(inputMessage.getBody()));
6366
}
6467

6568
@Override
66-
protected final void writeInternal(T t, HttpOutputMessage outputMessage) throws IOException {
69+
protected final void writeInternal(T t, HttpOutputMessage outputMessage)
70+
throws IOException, HttpMessageNotWritableException {
71+
6772
writeToResult(t, outputMessage.getHeaders(), new StreamResult(outputMessage.getBody()));
6873
}
6974

@@ -85,20 +90,20 @@ protected void transform(Source source, Result result) throws TransformerExcepti
8590
* @param source the HTTP input body
8691
* @return the converted object
8792
* @throws IOException in case of I/O errors
88-
* @throws org.springframework.http.converter.HttpMessageConversionException in case of conversion errors
93+
* @throws HttpMessageNotReadableException in case of conversion errors
8994
*/
9095
protected abstract T readFromSource(Class<? extends T> clazz, HttpHeaders headers, Source source)
91-
throws IOException;
96+
throws IOException, HttpMessageNotReadableException;
9297

9398
/**
9499
* Abstract template method called from {@link #writeInternal(Object, HttpOutputMessage)}.
95100
* @param t the object to write to the output message
96101
* @param headers the HTTP output headers
97102
* @param result the HTTP output body
98103
* @throws IOException in case of I/O errors
99-
* @throws HttpMessageConversionException in case of conversion errors
104+
* @throws HttpMessageNotWritableException in case of conversion errors
100105
*/
101106
protected abstract void writeToResult(T t, HttpHeaders headers, Result result)
102-
throws IOException;
107+
throws IOException, HttpMessageNotWritableException;
103108

104109
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,21 +160,21 @@ else if (elementClass.isAnnotationPresent(XmlType.class)) {
160160
}
161161
else {
162162
// should not happen, since we check in canRead(Type)
163-
throw new HttpMessageConversionException("Could not unmarshal to [" + elementClass + "]");
163+
throw new HttpMessageNotReadableException("Cannot unmarshal to [" + elementClass + "]");
164164
}
165165
event = moveToNextElement(streamReader);
166166
}
167167
return result;
168168
}
169+
catch (XMLStreamException ex) {
170+
throw new HttpMessageNotReadableException("Failed to read XML stream: " + ex.getMessage(), ex);
171+
}
169172
catch (UnmarshalException ex) {
170173
throw new HttpMessageNotReadableException(
171174
"Could not unmarshal to [" + elementClass + "]: " + ex.getMessage(), ex);
172175
}
173176
catch (JAXBException ex) {
174-
throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
175-
}
176-
catch (XMLStreamException ex) {
177-
throw new HttpMessageConversionException(ex.getMessage(), ex);
177+
throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
178178
}
179179
}
180180

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -145,10 +145,9 @@ protected Object readFromSource(Class<?> clazz, HttpHeaders headers, Source sour
145145
}
146146
catch (UnmarshalException ex) {
147147
throw new HttpMessageNotReadableException("Could not unmarshal to [" + clazz + "]: " + ex.getMessage(), ex);
148-
149148
}
150149
catch (JAXBException ex) {
151-
throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
150+
throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
152151
}
153152
}
154153

@@ -189,7 +188,7 @@ protected void writeToResult(Object o, HttpHeaders headers, Result result) throw
189188
throw new HttpMessageNotWritableException("Could not marshal [" + o + "]: " + ex.getMessage(), ex);
190189
}
191190
catch (JAXBException ex) {
192-
throw new HttpMessageConversionException("Could not instantiate JAXBContext: " + ex.getMessage(), ex);
191+
throw new HttpMessageConversionException("Invalid JAXB setup: " + ex.getMessage(), ex);
193192
}
194193
}
195194

0 commit comments

Comments
 (0)