diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartParser.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartParser.java
index 59839b1f5fb..42b3aec9e6b 100644
--- a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartParser.java
+++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartParser.java
@@ -16,7 +16,6 @@
package org.springframework.http.converter.multipart;
-import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
@@ -81,10 +80,28 @@ final class MultipartParser {
}
+ /**
+ * Simple delegation to {@link State#data(DataBuffer)}.
+ */
void handleData(DataBuffer dataBuffer) {
this.state.data(dataBuffer);
}
+ /**
+ * Handle a parsing error cleaning resources in the state and in the listener
+ * unless the error is {@link HttpMessageConversionException} in which case
+ * it is simply propagated.
+ */
+ void handleException(String message, @Nullable Throwable cause) {
+ if (cause instanceof HttpMessageConversionException ex) {
+ throw ex;
+ }
+ changeState(DisposedState.INSTANCE, null);
+ HttpMessageConversionException ex = new HttpMessageConversionException(message, cause);
+ this.listener.onError(ex);
+ throw ex;
+ }
+
private void changeState(State newState, @Nullable DataBuffer remainder) {
if (logger.isTraceEnabled()) {
logger.trace("Changed state: " + this.state + " -> " + newState);
@@ -118,6 +135,7 @@ final class MultipartParser {
return result;
}
+
/**
* Parse the given stream of bytes into events published to the given {@link PartListener}.
* @param input the input stream
@@ -141,9 +159,8 @@ final class MultipartParser {
}
parser.state.complete();
}
- catch (IOException ex) {
- parser.state.dispose();
- listener.onError(new HttpMessageConversionException("Could not decode multipart message", ex));
+ catch (Throwable ex) {
+ parser.handleException("Could not decode multipart message", ex);
}
}
@@ -155,13 +172,19 @@ final class MultipartParser {
/**
* Handle {@link HttpHeaders} for a part.
+ * Expectations for exception handling are the same as for {@link #onBody}.
*/
void onHeaders(HttpHeaders headers);
/**
- * Handle a piece of data for a body part.
+ * Handle the next chunk of body data.
+ *
Implementations must release the input buffer.
+ *
Implementations must handle all exceptions, cleaning up resources,
+ * and wrapping the exception as {@link HttpMessageConversionException}.
* @param buffer a chunk of body
- * @param last whether this is the last chunk for the part
+ * @param last whether this is the last chunk for the part
+ * @throws HttpMessageConversionException if the buffer could not be
+ * handled due to exceeded limits or for any other reason
*/
void onBody(DataBuffer buffer, boolean last);
@@ -171,7 +194,12 @@ final class MultipartParser {
void onComplete();
/**
- * Handle any error thrown during the parsing phase.
+ * Handle any error thrown during the parsing phase. The purpose of the
+ * method call is to allow cleaning up of resources. The listener does
+ * not need to throw or wrap and throw the error.
+ *
{@link #onHeaders} and {@link #onBody} are expected to handle their
+ * own exceptions, i.e. any exception those methods throw will not be
+ * passed here.
*/
void onError(Throwable error);
@@ -194,10 +222,26 @@ final class MultipartParser {
*/
private interface State {
+ /**
+ * Handle the next chunk of data.
+ *
If this method raises any exception other than
+ * {@link HttpMessageConversionException}, it will be
+ * {@link MultipartParser#handleException handled} by the parser.
+ * An {@link HttpMessageConversionException} on the other hand is
+ * considered fully handled and allowed to propagate as is.
+ */
void data(DataBuffer buf);
+ /**
+ * Called when the current part is fully parsed.
+ *
Expecations for exception handling are the same as for {@link #data}.
+ */
void complete();
+ /**
+ * Clean up resources held by the state. Called in case of errors or
+ * when switching to a new state.
+ */
default void dispose() {
}
@@ -241,8 +285,7 @@ final class MultipartParser {
@Override
public void complete() {
- changeState(DisposedState.INSTANCE, null);
- MultipartParser.this.listener.onError(new HttpMessageConversionException("Could not find first boundary"));
+ handleException("Could not find first boundary", null);
}
@Override
@@ -312,7 +355,14 @@ final class MultipartParser {
if (logger.isTraceEnabled()) {
logger.trace("Emitting headers: " + headers);
}
- MultipartParser.this.listener.onHeaders(headers);
+ try {
+ MultipartParser.this.listener.onHeaders(headers);
+ }
+ catch (Throwable ex) {
+ // PartListener should have cleaned its state, clean our own
+ dispose();
+ throw ex;
+ }
}
/**
@@ -338,12 +388,9 @@ final class MultipartParser {
if (count <= MultipartParser.this.maxHeadersSize) {
return true;
}
- else {
- MultipartParser.this.listener.onError(
- new HttpMessageConversionException("Part headers exceeded the memory usage limit of " +
- MultipartParser.this.maxHeadersSize + " bytes"));
- return false;
- }
+ MultipartParser.this.handleException(
+ "Part headers exceeded the limit of " + MultipartParser.this.maxHeadersSize + " bytes", null);
+ return false;
}
/**
@@ -377,8 +424,7 @@ final class MultipartParser {
@Override
public void complete() {
- changeState(DisposedState.INSTANCE, null);
- MultipartParser.this.listener.onError(new HttpMessageConversionException("Could not find end of headers"));
+ MultipartParser.this.handleException("Could not find end of headers", null);
}
@Override
@@ -493,25 +539,32 @@ final class MultipartParser {
}
len += previous.readableByteCount();
}
- emit.forEach(buffer -> MultipartParser.this.listener.onBody(buffer, false));
+ emit.forEach(buffer -> invokeListener(buffer, false));
}
private void flush() {
for (Iterator iterator = this.queue.iterator(); iterator.hasNext(); ) {
DataBuffer buffer = iterator.next();
boolean last = !iterator.hasNext();
- MultipartParser.this.listener.onBody(buffer, last);
+ invokeListener(buffer, last);
}
this.queue.clear();
}
+ private void invokeListener(DataBuffer buffer, boolean last) {
+ try {
+ MultipartParser.this.listener.onBody(buffer, last);
+ }
+ catch (Throwable ex) {
+ dispose();
+ throw ex;
+ }
+ }
+
@Override
public void complete() {
- changeState(DisposedState.INSTANCE, null);
- String msg = "Could not find end of body (␍␊--" +
- new String(MultipartParser.this.boundary, StandardCharsets.UTF_8) +
- ")";
- MultipartParser.this.listener.onError(new HttpMessageConversionException(msg));
+ MultipartParser.this.handleException("Could not find end of body (␍␊--" +
+ new String(MultipartParser.this.boundary, StandardCharsets.UTF_8) + ")", null);
}
@Override
diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/PartGenerator.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/PartGenerator.java
index 90757db2eca..5e1046e13ae 100644
--- a/spring-web/src/main/java/org/springframework/http/converter/multipart/PartGenerator.java
+++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/PartGenerator.java
@@ -96,8 +96,8 @@ final class PartGenerator implements MultipartParser.PartListener {
private static boolean isFormField(HttpHeaders headers) {
MediaType contentType = headers.getContentType();
- return (contentType == null || MediaType.TEXT_PLAIN.equalsTypeAndSubtype(contentType)) &&
- headers.getContentDisposition().getFilename() == null;
+ return ((contentType == null || MediaType.TEXT_PLAIN.equalsTypeAndSubtype(contentType)) &&
+ headers.getContentDisposition().getFilename() == null);
}
@Override
@@ -133,9 +133,8 @@ final class PartGenerator implements MultipartParser.PartListener {
}
@Override
- public void onError(Throwable error) {
+ public void onError(Throwable ex) {
deleteParts();
- throw new HttpMessageConversionException("Cannot decode multipart body", error);
}
void addPart(Part part) {
@@ -168,7 +167,7 @@ final class PartGenerator implements MultipartParser.PartListener {
private interface State {
/**
- * Invoked when a {@link MultipartParser.PartListener#onBody(DataBuffer, boolean)} is received.
+ * Invoked when the parser receives additional data.
*/
void onBody(DataBuffer dataBuffer, boolean last);
diff --git a/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverterTests.java
index 8cbc5988ad7..14200fd12ae 100644
--- a/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverterTests.java
+++ b/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverterTests.java
@@ -200,7 +200,7 @@ class MultipartHttpMessageConverterTests {
void readMultipartInvalid() throws Exception {
MockHttpInputMessage response = createMultipartResponse("garbage-1.multipart", "boundary");
assertThatThrownBy(() -> converter.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Part.class), response, null))
- .isInstanceOf(HttpMessageConversionException.class).hasMessage("Cannot decode multipart body");
+ .isInstanceOf(HttpMessageConversionException.class).hasMessage("Could not find first boundary");
}
@Test
diff --git a/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartParserTests.java b/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartParserTests.java
index 067ee7cd771..d324cb117e3 100644
--- a/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartParserTests.java
+++ b/spring-web/src/test/java/org/springframework/http/converter/multipart/MultipartParserTests.java
@@ -38,6 +38,7 @@ import org.springframework.http.converter.HttpMessageConversionException;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for {@link MultipartParser}.
@@ -74,7 +75,9 @@ class MultipartParserTests {
@Test
void noEndBoundary() throws Exception {
TestListener listener = new TestListener();
- parse("no-end-boundary.multipart", "boundary", listener);
+ assertThatThrownBy(() -> parse("no-end-boundary.multipart", "boundary", listener))
+ .isInstanceOf(HttpMessageConversionException.class)
+ .hasMessage("Could not find end of headers");
assertThat(listener.error).isInstanceOf(HttpMessageConversionException.class);
}
@@ -82,7 +85,9 @@ class MultipartParserTests {
@Test
void garbage() throws Exception {
TestListener listener = new TestListener();
- parse("garbage-1.multipart", "boundary", listener);
+ assertThatThrownBy(() -> parse("garbage-1.multipart", "boundary", listener))
+ .isInstanceOf(HttpMessageConversionException.class)
+ .hasMessage("Could not find first boundary");
assertThat(listener.error).isInstanceOf(HttpMessageConversionException.class);
}
@@ -90,7 +95,9 @@ class MultipartParserTests {
@Test
void noEndHeader() throws Exception {
TestListener listener = new TestListener();
- parse("no-end-header.multipart", "boundary", listener);
+ assertThatThrownBy(() -> parse("no-end-header.multipart", "boundary", listener))
+ .isInstanceOf(HttpMessageConversionException.class)
+ .hasMessage("Could not find end of headers");
assertThat(listener.error).isInstanceOf(HttpMessageConversionException.class);
}
@@ -98,7 +105,9 @@ class MultipartParserTests {
@Test
void noEndBody() throws Exception {
TestListener listener = new TestListener();
- parse("no-end-body.multipart", "boundary", listener);
+ assertThatThrownBy(() -> parse("no-end-body.multipart", "boundary", listener))
+ .isInstanceOf(HttpMessageConversionException.class)
+ .hasMessage("Could not find end of body (␍␊--boundary)");
assertThat(listener.error).isInstanceOf(HttpMessageConversionException.class);
}