From 8cfe90c4c0768a6beabc98dbd637ed78086c1260 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 17 Jun 2026 10:13:47 +0100 Subject: [PATCH] Polishing in MultipartHttpMessageConverter --- .../converter/multipart/DefaultParts.java | 1 + .../MultipartHttpMessageConverter.java | 72 +++++++++++-------- .../converter/multipart/MultipartParser.java | 4 ++ .../converter/multipart/MultipartUtils.java | 1 + .../http/converter/multipart/Part.java | 1 - .../converter/multipart/PartGenerator.java | 53 ++++++++------ .../converter/multipart/package-info.java | 3 +- 7 files changed, 82 insertions(+), 53 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/DefaultParts.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/DefaultParts.java index 293ef04f178..318f6923232 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/multipart/DefaultParts.java +++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/DefaultParts.java @@ -34,6 +34,7 @@ import org.springframework.util.Assert; * * @author Arjen Poutsma * @author Brian Clozel + * @since 7.1 */ abstract class DefaultParts { diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverter.java index daf70768dd5..252cbf0d479 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartHttpMessageConverter.java @@ -158,6 +158,7 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< private int maxParts = -1; + /** * Create a new converter instance with the given converter instances for reading and * writing parts. @@ -185,6 +186,7 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< new ResourceHttpMessageConverter())); } + /** * Set the list of {@link MediaType} objects supported by this converter. * @see #addSupportedMediaTypes(MediaType...) @@ -219,7 +221,6 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< return Collections.unmodifiableList(this.supportedMediaTypes); } - /** * Return the configured converters for MIME parts. */ @@ -274,7 +275,6 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< this.multipartCharset = charset; } - /** * Configure the maximum amount of memory that is allowed per headers section of each part. *

By default, this is set to 10K. @@ -317,17 +317,15 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< this.maxParts = maxParts; } + @Override public boolean canRead(ResolvableType elementType, @Nullable MediaType mediaType) { if (!supportsMediaType(mediaType)) { return false; } - if (!MultiValueMap.class.isAssignableFrom(elementType.toClass()) || - (!elementType.hasUnresolvableGenerics() && - !Part.class.isAssignableFrom(elementType.getGeneric(1).toClass()))) { - return false; - } - return true; + return (MultiValueMap.class.isAssignableFrom(elementType.toClass()) && + (elementType.hasUnresolvableGenerics() || + Part.class.isAssignableFrom(elementType.getGeneric(1).toClass()))); } private boolean supportsMediaType(@Nullable MediaType mediaType) { @@ -343,7 +341,9 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< } @Override - public MultiValueMap read(ResolvableType type, HttpInputMessage message, @Nullable Map hints) throws IOException, HttpMessageNotReadableException { + public MultiValueMap read( + ResolvableType type, HttpInputMessage message, @Nullable Map hints) + throws IOException, HttpMessageNotReadableException { Charset headersCharset = MultipartUtils.charset(message.getHeaders()); byte[] boundary = boundary(message, headersCharset); @@ -351,12 +351,15 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< throw new HttpMessageNotReadableException("No multipart boundary found in Content-Type: \"" + message.getHeaders().getContentType() + "\"", message); } - PartGenerator partListener = new PartGenerator(this.maxInMemorySize, this.maxDiskUsagePerPart, this.maxParts, getTempDirectory()); - new MultipartParser(this.maxHeadersSize, 2 * 1024).parse(message.getBody(), boundary, - headersCharset, partListener); - return partListener.getParts(); - } + PartGenerator partGenerator = new PartGenerator( + this.maxInMemorySize, this.maxDiskUsagePerPart, this.maxParts, getTempDirectory()); + + MultipartParser parser = new MultipartParser(this.maxHeadersSize, 2 * 1024); + parser.parse(message.getBody(), boundary, headersCharset, partGenerator); + + return partGenerator.getParts(); + } private static byte @Nullable [] boundary(HttpInputMessage message, Charset headersCharset) { MediaType contentType = message.getHeaders().getContentType(); @@ -398,7 +401,11 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< @Override @SuppressWarnings("unchecked") - public void write(MultiValueMap map, ResolvableType type, @Nullable MediaType contentType, HttpOutputMessage outputMessage, @Nullable Map hints) throws IOException, HttpMessageNotWritableException { + public void write( + MultiValueMap map, ResolvableType type, @Nullable MediaType contentType, + HttpOutputMessage outputMessage, @Nullable Map hints) + throws IOException, HttpMessageNotWritableException { + MultiValueMap parts = (MultiValueMap) map; // If the supplied content type is null, fall back to multipart/form-data. @@ -446,19 +453,25 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< } - @SuppressWarnings({"unchecked", "ConstantValue"}) + @SuppressWarnings("ConstantValue") private boolean checkPartsRepeatable(MultiValueMap map, MediaType contentType) { - return map.entrySet().stream().allMatch(e -> e.getValue().stream().filter(Objects::nonNull).allMatch(part -> { - HttpHeaders headers = null; - Object body = part; - if (part instanceof HttpEntity entity) { - headers = entity.getHeaders(); - body = entity.getBody(); - Assert.state(body != null, "Empty body for part '" + e.getKey() + "': " + part); - } - HttpMessageConverter converter = (HttpMessageConverter) findConverterFor(e.getKey(), headers, body); - return converter != null && converter.canWriteRepeatedly((T) body, contentType); - })); + return map.entrySet().stream().allMatch(entry -> + entry.getValue().stream() + .filter(Objects::nonNull) + .allMatch(part -> isPartRepeatable(entry.getKey(), part, contentType))); + } + + @SuppressWarnings("unchecked") + private boolean isPartRepeatable(String name, Object part, MediaType contentType) { + HttpHeaders headers = null; + Object body = part; + if (part instanceof HttpEntity entity) { + headers = entity.getHeaders(); + body = entity.getBody(); + Assert.state(body != null, "Empty body for part '" + name + "': " + part); + } + HttpMessageConverter converter = (HttpMessageConverter) findConverterFor(name, headers, body); + return (converter != null && converter.canWriteRepeatedly((T) body, contentType)); } private @Nullable HttpMessageConverter findConverterFor( @@ -483,7 +496,9 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< return (this.multipartCharset != null); } - private void writeParts(OutputStream os, MultiValueMap parts, byte[] boundary) throws IOException { + private void writeParts( + OutputStream os, MultiValueMap parts, byte[] boundary) throws IOException { + for (Map.Entry> entry : parts.entrySet()) { String name = entry.getKey(); for (Object part : entry.getValue()) { @@ -549,7 +564,6 @@ public class MultipartHttpMessageConverter implements SmartHttpMessageConverter< } } - private void writeBoundary(OutputStream os, byte[] boundary) throws IOException { os.write('-'); os.write('-'); 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 194ea1631c4..4d9b3deb45a 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 @@ -43,11 +43,13 @@ import org.springframework.http.converter.HttpMessageConversionException; * * @author Brian Clozel * @author Arjen Poutsma + * @since 7.1 */ final class MultipartParser { private static final Log logger = LogFactory.getLog(MultipartParser.class); + private final int maxHeadersSize; private final int bufferSize; @@ -89,6 +91,7 @@ final class MultipartParser { } } + private final class InternalParser { private final byte[] boundary; @@ -139,6 +142,7 @@ final class MultipartParser { return result; } + /** * Represents the internal state of the {@link MultipartParser}. * The flow for well-formed multipart messages is shown below: diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartUtils.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartUtils.java index 37a45a77862..288c15d99d1 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartUtils.java +++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/MultipartUtils.java @@ -26,6 +26,7 @@ import org.springframework.http.MediaType; * Various static utility methods for dealing with multipart parsing. * @author Arjen Poutsma * @author Brian Clozel + * @since 7.1 */ abstract class MultipartUtils { diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/Part.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/Part.java index a0a8f7fe83c..397314e6bb0 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/multipart/Part.java +++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/Part.java @@ -63,7 +63,6 @@ public interface Part { * Delete the underlying storage for this part. */ default void delete() throws IOException { - } } 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 9eb43d05ca5..c4dff4c7ace 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 @@ -47,11 +47,13 @@ import org.springframework.util.MultiValueMap; * * @author Brian Clozel * @author Arjen Poutsma + * @since 7.1 */ final class PartGenerator implements MultipartParser.PartListener { private static final Log logger = LogFactory.getLog(PartGenerator.class); + private final MultiValueMap parts = new LinkedMultiValueMap<>(); private final int maxInMemorySize; @@ -266,23 +268,26 @@ final class PartGenerator implements MultipartParser.PartListener { @Override public void onBody(DataBuffer dataBuffer, boolean last) { this.byteCount += dataBuffer.readableByteCount(); - if (PartGenerator.this.maxInMemorySize == -1 || - this.byteCount <= PartGenerator.this.maxInMemorySize) { - this.content.add(dataBuffer); - if (last) { - emitMemoryPart(); - } - } - else { + if (isMaxInMemorySizeExceeded()) { switchToFile(dataBuffer, last); + return; + } + this.content.add(dataBuffer); + if (last) { + emitMemoryPart(); } } + private boolean isMaxInMemorySizeExceeded() { + return (PartGenerator.this.maxInMemorySize != -1 && + this.byteCount > PartGenerator.this.maxInMemorySize); + } + private void switchToFile(DataBuffer current, boolean last) { - FileState newState = new FileState(this.headers, PartGenerator.this.fileStorageDirectory); - this.content.forEach(newState::writeBuffer); - newState.onBody(current, last); - PartGenerator.this.state = newState; + FileState fileState = new FileState(this.headers, PartGenerator.this.fileStorageDirectory); + this.content.forEach(fileState::writeBuffer); + fileState.onBody(current, last); + PartGenerator.this.state = fileState; } private void emitMemoryPart() { @@ -331,23 +336,27 @@ final class PartGenerator implements MultipartParser.PartListener { @Override public void onBody(DataBuffer dataBuffer, boolean last) { this.byteCount += dataBuffer.readableByteCount(); - if (PartGenerator.this.maxDiskUsagePerPart == -1 || this.byteCount <= PartGenerator.this.maxDiskUsagePerPart) { - writeBuffer(dataBuffer); - if (last) { - Part part = DefaultParts.part(this.headers, this.file); - PartGenerator.this.addPart(part); - } - } - else { + if (isMaxDiskUsagePerPartExceeded()) { try { this.outputStream.close(); } catch (IOException exc) { // ignored } - throw new HttpMessageConversionException("Part exceeded the disk usage limit of " + - PartGenerator.this.maxDiskUsagePerPart + " bytes"); + throw new HttpMessageConversionException( + "Part exceeded the disk usage limit of " + + PartGenerator.this.maxDiskUsagePerPart + " bytes"); } + writeBuffer(dataBuffer); + if (last) { + Part part = DefaultParts.part(this.headers, this.file); + PartGenerator.this.addPart(part); + } + } + + private boolean isMaxDiskUsagePerPartExceeded() { + return (PartGenerator.this.maxDiskUsagePerPart != -1 && + this.byteCount > PartGenerator.this.maxDiskUsagePerPart); } private Path createFile(Path directory) { diff --git a/spring-web/src/main/java/org/springframework/http/converter/multipart/package-info.java b/spring-web/src/main/java/org/springframework/http/converter/multipart/package-info.java index 158445f2c3e..253c2b4a414 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/multipart/package-info.java +++ b/spring-web/src/main/java/org/springframework/http/converter/multipart/package-info.java @@ -1,5 +1,6 @@ /** - * Provides an HttpMessageConverter for Multipart support. + * Provides support for reading and writing multipart support, + * through an {@link org.springframework.http.converter.HttpMessageConverter}. */ @NullMarked package org.springframework.http.converter.multipart;