From 6e9a19212f440d1aa2c6e14c67c2771c33cf5b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 23 Jul 2024 10:39:57 +0200 Subject: [PATCH] Consistently check for Content-Length value This commit makes sure to consistently check that the content length is not set above 2GB. Previously it was only checked in setContentLength. Closes gh-33256 --- .../util/ContentCachingResponseWrapper.java | 18 +++++---- .../ContentCachingResponseWrapperTests.java | 38 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java b/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java index c2038fb0ecc2..41fc196d6781 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java +++ b/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java @@ -140,11 +140,15 @@ public void setContentLength(int len) { @Override public void setContentLengthLong(long len) { - if (len > Integer.MAX_VALUE) { + setContentLength(toContentLengthInt(len)); + } + + private int toContentLengthInt(long contentLength) { + if (contentLength > Integer.MAX_VALUE) { throw new IllegalArgumentException("Content-Length exceeds ContentCachingResponseWrapper's maximum (" + - Integer.MAX_VALUE + "): " + len); + Integer.MAX_VALUE + "): " + contentLength); } - setContentLength((int) len); + return (int) contentLength; } @Override @@ -160,7 +164,7 @@ public boolean containsHeader(String name) { @Override public void setHeader(String name, String value) { if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { - this.contentLength = Integer.valueOf(value); + this.contentLength = toContentLengthInt(Long.parseLong(value)); } else { super.setHeader(name, value); @@ -170,7 +174,7 @@ public void setHeader(String name, String value) { @Override public void addHeader(String name, String value) { if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { - this.contentLength = Integer.valueOf(value); + this.contentLength = toContentLengthInt(Long.parseLong(value)); } else { super.addHeader(name, value); @@ -180,7 +184,7 @@ public void addHeader(String name, String value) { @Override public void setIntHeader(String name, int value) { if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { - this.contentLength = Integer.valueOf(value); + this.contentLength = value; } else { super.setIntHeader(name, value); @@ -190,7 +194,7 @@ public void setIntHeader(String name, int value) { @Override public void addIntHeader(String name, int value) { if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { - this.contentLength = Integer.valueOf(value); + this.contentLength = value; } else { super.addIntHeader(name, value); diff --git a/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java b/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java index d5f9c39918bc..25ddffbc3b63 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java @@ -31,6 +31,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Named.named; import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; import static org.springframework.http.HttpHeaders.CONTENT_TYPE; @@ -233,6 +234,43 @@ void copyBodyToResponseWithTransferEncoding() throws Exception { assertThat(response.getContentAsByteArray()).isEqualTo(responseBody); } + @Test + void setContentLengthAbove2GbViaSetContentLengthLong() { + MockHttpServletResponse response = new MockHttpServletResponse(); + + ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response); + long overflow = (long) Integer.MAX_VALUE + 1; + assertThatIllegalArgumentException() + .isThrownBy(() -> responseWrapper.setContentLengthLong(overflow)) + .withMessageContaining("Content-Length exceeds ContentCachingResponseWrapper's maximum") + .withMessageContaining(String.valueOf(overflow)); + } + + @Test + void setContentLengthAbove2GbViaAddHeader() { + MockHttpServletResponse response = new MockHttpServletResponse(); + + ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response); + String overflow = String.valueOf((long) Integer.MAX_VALUE + 1); + assertThatIllegalArgumentException() + .isThrownBy(() -> responseWrapper.addHeader(CONTENT_LENGTH, overflow)) + .withMessageContaining("Content-Length exceeds ContentCachingResponseWrapper's maximum") + .withMessageContaining(overflow); + } + + @Test + void setContentLengthAbove2GbViaSetHeader() { + MockHttpServletResponse response = new MockHttpServletResponse(); + + ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response); + String overflow = String.valueOf((long) Integer.MAX_VALUE + 1); + assertThatIllegalArgumentException() + .isThrownBy(() -> responseWrapper.setHeader(CONTENT_LENGTH, overflow)) + .withMessageContaining("Content-Length exceeds ContentCachingResponseWrapper's maximum") + .withMessageContaining(overflow); + } + + private void assertHeader(HttpServletResponse response, String header, int value) { assertHeader(response, header, Integer.toString(value)); }