diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java index d4248c8442d..6e94678cef1 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java @@ -63,7 +63,7 @@ public class DefaultPartHttpMessageReader extends LoggingCodecSupport implements private int maxInMemorySize = 256 * 1024; - private int maxHeadersSize = 8 * 1024; + private int maxHeadersSize = 10 * 1024; private long maxDiskUsagePerPart = -1; diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java index d2057f53d62..d797b99f4b1 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java @@ -342,33 +342,23 @@ final class MultipartParser extends BaseSubscriber { /** * First checks whether the multipart boundary leading to this state - * was the final boundary, or whether {@link #maxHeadersSize} is - * exceeded. Then looks for the header-body boundary - * ({@code CR LF CR LF}) in the given buffer. If found, convert - * all buffers collected so far into a {@link HttpHeaders} object + * was the final boundary. Then looks for the header-body boundary + * ({@code CR LF CR LF}) in the given buffer. If found, checks whether + * the size of all header buffers does not exceed {@link #maxHeadersSize}, + * converts all buffers collected so far into a {@link HttpHeaders} object * and changes to {@link BodyState}, passing the remainder of the - * buffer. If the boundary is not found, the buffer is collected. + * buffer. If the boundary is not found, the buffer is collected if + * its size does not exceed {@link #maxHeadersSize}. */ @Override public void onNext(DataBuffer buf) { - long prevCount = this.byteCount.get(); - long count = this.byteCount.addAndGet(buf.readableByteCount()); - if (prevCount < 2 && count >= 2) { - if (isLastBoundary(buf)) { - if (logger.isTraceEnabled()) { - logger.trace("Last boundary found in " + buf); - } - - if (changeState(this, DisposedState.INSTANCE, buf)) { - emitComplete(); - } - return; + if (isLastBoundary(buf)) { + if (logger.isTraceEnabled()) { + logger.trace("Last boundary found in " + buf); } - } - else if (count > MultipartParser.this.maxHeadersSize) { + if (changeState(this, DisposedState.INSTANCE, buf)) { - emitError(new DataBufferLimitException("Part headers exceeded the memory usage limit of " + - MultipartParser.this.maxHeadersSize + " bytes")); + emitComplete(); } return; } @@ -377,17 +367,23 @@ final class MultipartParser extends BaseSubscriber { if (logger.isTraceEnabled()) { logger.trace("End of headers found @" + endIdx + " in " + buf); } - DataBuffer headerBuf = MultipartUtils.sliceTo(buf, endIdx); - this.buffers.add(headerBuf); - DataBuffer bodyBuf = MultipartUtils.sliceFrom(buf, endIdx); - DataBufferUtils.release(buf); - - emitHeaders(parseHeaders()); - changeState(this, new BodyState(), bodyBuf); + long count = this.byteCount.addAndGet(endIdx); + if (belowMaxHeaderSize(count)) { + DataBuffer headerBuf = MultipartUtils.sliceTo(buf, endIdx); + this.buffers.add(headerBuf); + DataBuffer bodyBuf = MultipartUtils.sliceFrom(buf, endIdx); + DataBufferUtils.release(buf); + + emitHeaders(parseHeaders()); + changeState(this, new BodyState(), bodyBuf); + } } else { - this.buffers.add(buf); - requestBuffer(); + long count = this.byteCount.addAndGet(buf.readableByteCount()); + if (belowMaxHeaderSize(count)) { + this.buffers.add(buf); + requestBuffer(); + } } } @@ -407,6 +403,21 @@ final class MultipartParser extends BaseSubscriber { buf.getByte(0) == HYPHEN); } + /** + * Checks whether the given {@code count} is below or equal to {@link #maxHeadersSize} + * and emits a {@link DataBufferLimitException} if not. + */ + private boolean belowMaxHeaderSize(long count) { + if (count <= MultipartParser.this.maxHeadersSize) { + return true; + } + else { + emitError(new DataBufferLimitException("Part headers exceeded the memory usage limit of " + + MultipartParser.this.maxHeadersSize + " bytes")); + return false; + } + } + /** * Parses the list of buffers into a {@link HttpHeaders} instance. * Converts the joined buffers into a string using ISO=8859-1, and parses diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java index 8e812e720fb..9179a546820 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java @@ -270,6 +270,31 @@ public class DefaultPartHttpMessageReaderTests { latch.await(); } + // gh-27612 + @Test + public void exceedHeaderLimit() throws InterruptedException { + Flux body = DataBufferUtils + .readByteChannel((new ClassPathResource("files.multipart", getClass()))::readableChannel, bufferFactory, 282); + + MediaType contentType = new MediaType("multipart", "form-data", singletonMap("boundary", "----WebKitFormBoundaryG8fJ50opQOML0oGD")); + MockServerHttpRequest request = MockServerHttpRequest.post("/") + .contentType(contentType) + .body(body); + + DefaultPartHttpMessageReader reader = new DefaultPartHttpMessageReader(); + + reader.setMaxHeadersSize(230); + + Flux result = reader.read(forClass(Part.class), request, emptyMap()); + + CountDownLatch latch = new CountDownLatch(2); + StepVerifier.create(result) + .consumeNextWith(part -> testPart(part, null, LOREM_IPSUM, latch)) + .consumeNextWith(part -> testPart(part, null, MUSPI_MEROL, latch)) + .verifyComplete(); + + latch.await(); + } private void testBrowser(DefaultPartHttpMessageReader reader, Resource resource, String boundary) throws InterruptedException { diff --git a/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart b/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart index 03b41190647..bd10aea6b0a 100644 --- a/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart +++ b/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart @@ -3,11 +3,9 @@ Content-Disposition: form-data; name="file2"; filename="a.txt" Content-Type: text/plain Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer iaculis metus id vestibulum nullam. - ------WebKitFormBoundaryG8fJ50opQOML0oGD Content-Disposition: form-data; name="file2"; filename="b.txt" Content-Type: text/plain .mallun mulubitsev di sutem silucai regetnI .tile gnicsipida rutetcesnoc ,tema tis rolod muspi meroL - ------WebKitFormBoundaryG8fJ50opQOML0oGD--