From 0ca393c0dcfacf3d5a50f9d9a48432fba6a1cdbf Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 10 Jun 2024 09:46:01 +0200 Subject: [PATCH] Restrict memory allocation in ContentCachingRequestWrapper Prior to this commit, the `ContentCachingRequestWrapper` could allocate a `FastByteArrayOutputStream` block that was larger than the content cache limit given as a consturctor argument. This was due to an optimization applied in gh-31834 for allocating the right content cache size when the request size is known. Fixes gh-32987 --- .../web/util/ContentCachingRequestWrapper.java | 7 ++++++- .../util/ContentCachingRequestWrapperTests.java | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java b/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java index 6d0e1135071d..3821051fa7b7 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java +++ b/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java @@ -89,7 +89,12 @@ public ContentCachingRequestWrapper(HttpServletRequest request) { public ContentCachingRequestWrapper(HttpServletRequest request, int contentCacheLimit) { super(request); int contentLength = request.getContentLength(); - this.cachedContent = (contentLength > 0) ? new FastByteArrayOutputStream(contentLength) : new FastByteArrayOutputStream(); + if (contentLength > 0) { + this.cachedContent = new FastByteArrayOutputStream(Math.min(contentLength, contentCacheLimit)); + } + else { + this.cachedContent = new FastByteArrayOutputStream(); + } this.contentCacheLimit = contentCacheLimit; } diff --git a/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java b/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java index df5b778268fc..fe710e1695b1 100644 --- a/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java @@ -17,12 +17,15 @@ package org.springframework.web.util; import java.io.UnsupportedEncodingException; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.util.FastByteArrayOutputStream; +import org.springframework.util.ReflectionUtils; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -89,6 +92,19 @@ void cachedContentToStringWithLimit() throws Exception { assertThat(wrapper.getContentAsString()).isEqualTo(new String("Hel".getBytes(CHARSET), CHARSET)); } + @Test + void shouldNotAllocateMoreThanCacheLimit() throws Exception { + ContentCachingRequestWrapper wrapper = new ContentCachingRequestWrapper(createGetRequest("Hello World"), CONTENT_CACHE_LIMIT); + Field field = ReflectionUtils.findField(ContentCachingRequestWrapper.class, "cachedContent"); + ReflectionUtils.makeAccessible(field); + FastByteArrayOutputStream cachedContent = (FastByteArrayOutputStream) ReflectionUtils.getField(field, wrapper); + field = ReflectionUtils.findField(FastByteArrayOutputStream.class, "initialBlockSize"); + ReflectionUtils.makeAccessible(field); + int blockSize = (int) ReflectionUtils.getField(field, cachedContent); + assertThat(blockSize).isEqualTo(CONTENT_CACHE_LIMIT); + } + + @Test void cachedContentWithOverflow() throws Exception { ContentCachingRequestWrapper wrapper = new ContentCachingRequestWrapper(