From aaab5d2e1a0b80022c257d02ee88096963eb21d1 Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Wed, 4 Sep 2024 16:05:39 +0200 Subject: [PATCH 1/8] Add a new annotation and an interceptor to control and check content size of bodies and files --- .../control/ContentControlFeature.java | 149 ++++++++++++++++++ .../security/control/ContentSizeLimit.java | 24 +++ 2 files changed, 173 insertions(+) create mode 100644 plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java create mode 100644 plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentSizeLimit.java diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java new file mode 100644 index 0000000..cfccc21 --- /dev/null +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -0,0 +1,149 @@ +package com.coreoz.plume.jersey.security.control; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.AnnotatedElement; + +import javax.inject.Singleton; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.container.DynamicFeature; +import javax.ws.rs.container.ResourceInfo; +import javax.ws.rs.core.FeatureContext; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ReaderInterceptor; +import javax.ws.rs.ext.ReaderInterceptorContext; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Singleton +public class ContentControlFeature implements DynamicFeature { + + private static final Logger logger = LoggerFactory.getLogger(ContentControlFeature.class); + + @Override + public void configure(ResourceInfo resourceInfo, FeatureContext context) { + addContentSizeFilter(resourceInfo.getResourceMethod(), context); + } + + private void addContentSizeFilter(AnnotatedElement annotatedElement, FeatureContext methodResourcecontext) { + ContentSizeLimit contentSizeLimit = annotatedElement.getAnnotation(ContentSizeLimit.class); + methodResourcecontext.register(new ContentSizeLimitInterceptor( + contentSizeLimit != null ? contentSizeLimit.value() : ContentSizeLimitInterceptor.DEFAULT_MAX_SIZE + )); + } + + private static class ContentSizeLimitInterceptor implements ReaderInterceptor { + + private static final int DEFAULT_MAX_SIZE = 500; + + private final int maxSize; + + public ContentSizeLimitInterceptor(int maxSize) { + this.maxSize = maxSize; + } + + // https://stackoverflow.com/questions/24516444/best-way-to-make-jersey-2-x-refuse-requests-with-incorrect-content-length + @Override + public Object aroundReadFrom(ReaderInterceptorContext context) throws IOException { + final InputStream contextInputStream = context.getInputStream(); + final String headerContentLength = context.getHeaders().getFirst("Content-Length"); + final Long declaredContentLength = headerContentLength == null ? -1 : Long.valueOf(headerContentLength); + + context.setInputStream(new InputStream() { + private long length = 0; + private int mark = 0; + + @Override + public int read() throws IOException { + final int read = contextInputStream.read(); + readAndCheck(read != -1 ? 1 : 0); + return read; + } + + @Override + public int read(final byte[] b) throws IOException { + final int read = contextInputStream.read(b); + readAndCheck(read != -1 ? read : 0); + return read; + } + + @Override + public int read(final byte[] b, final int off, final int len) throws IOException { + final int read = contextInputStream.read(b, off, len); + readAndCheck(read != -1 ? read : 0); + return read; + } + + @Override + public long skip(final long n) throws IOException { + final long skip = contextInputStream.skip(n); + readAndCheck(skip != -1 ? skip : 0); + return skip; + } + + @Override + public int available() throws IOException { + return contextInputStream.available(); + } + + @Override + public void close() throws IOException { + contextInputStream.close(); + } + + @Override + public synchronized void mark(final int readlimit) { + mark += readlimit; + contextInputStream.mark(readlimit); + } + + @Override + public synchronized void reset() throws IOException { + this.length = 0; + readAndCheck(mark); + contextInputStream.reset(); + } + + @Override + public boolean markSupported() { + return contextInputStream.markSupported(); + } + + private void readAndCheck(final long read) { + this.length += read; + + if (this.length > declaredContentLength) { + try { + this.close(); + } catch (IOException e) { + logger.error("Error while closing the input stream", e); + } + throw new WebApplicationException( + Response.status(Response.Status.LENGTH_REQUIRED) + .entity("Incorrect content-length provided.") + .build() + ); + } + if (this.length > maxSize) { + try { + this.close(); + } catch (IOException e) { + logger.error("Error while closing the input stream", e); + } + throw new WebApplicationException( + Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) + .entity("Content size limit exceeded.") + .build() + ); + } + } + }); + + final Object entity = context.proceed(); + context.setInputStream(contextInputStream); + + return entity; + } + } +} diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentSizeLimit.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentSizeLimit.java new file mode 100644 index 0000000..3f9a66f --- /dev/null +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentSizeLimit.java @@ -0,0 +1,24 @@ +package com.coreoz.plume.jersey.security.control; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + + +/** + * Modify the default Content size limit handled by the backend + */ +@Documented +@Retention (RUNTIME) +@Target({TYPE, METHOD}) +public @interface ContentSizeLimit { + + /** + * The maximum size of the content + */ + int value(); +} From df2adc6c22081befbbb1eae434e4cb416784b86a Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Thu, 5 Sep 2024 13:33:05 +0200 Subject: [PATCH 2/8] Add unit test for contentControlFeature --- plume-web-jersey/pom.xml | 24 ++- .../control/ContentControlFeature.java | 191 ++++++++++-------- .../plume/jersey/WebJerseyTestModule.java | 11 + .../jersey/control/ContentSizeLimitTest.java | 101 +++++++++ 4 files changed, 237 insertions(+), 90 deletions(-) create mode 100644 plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java create mode 100644 plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java diff --git a/plume-web-jersey/pom.xml b/plume-web-jersey/pom.xml index 669d4cb..9b632e3 100644 --- a/plume-web-jersey/pom.xml +++ b/plume-web-jersey/pom.xml @@ -95,6 +95,28 @@ 3.1.0 provided + + + + com.carlosbecker + guice-junit-test-runner + + + guice + com.google.inject + + + + + junit + junit + test + + + org.assertj + assertj-core + test + @@ -109,4 +131,4 @@ - \ No newline at end of file + diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index cfccc21..d6f8e52 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -33,7 +33,7 @@ private void addContentSizeFilter(AnnotatedElement annotatedElement, FeatureCont )); } - private static class ContentSizeLimitInterceptor implements ReaderInterceptor { + public static class ContentSizeLimitInterceptor implements ReaderInterceptor { private static final int DEFAULT_MAX_SIZE = 500; @@ -50,100 +50,113 @@ public Object aroundReadFrom(ReaderInterceptorContext context) throws IOExceptio final String headerContentLength = context.getHeaders().getFirst("Content-Length"); final Long declaredContentLength = headerContentLength == null ? -1 : Long.valueOf(headerContentLength); - context.setInputStream(new InputStream() { - private long length = 0; - private int mark = 0; + context.setInputStream(new ContentSizeLimitInputStream(contextInputStream, declaredContentLength, maxSize)); - @Override - public int read() throws IOException { - final int read = contextInputStream.read(); - readAndCheck(read != -1 ? 1 : 0); - return read; - } - - @Override - public int read(final byte[] b) throws IOException { - final int read = contextInputStream.read(b); - readAndCheck(read != -1 ? read : 0); - return read; - } - - @Override - public int read(final byte[] b, final int off, final int len) throws IOException { - final int read = contextInputStream.read(b, off, len); - readAndCheck(read != -1 ? read : 0); - return read; - } - - @Override - public long skip(final long n) throws IOException { - final long skip = contextInputStream.skip(n); - readAndCheck(skip != -1 ? skip : 0); - return skip; - } - - @Override - public int available() throws IOException { - return contextInputStream.available(); - } - - @Override - public void close() throws IOException { - contextInputStream.close(); - } - - @Override - public synchronized void mark(final int readlimit) { - mark += readlimit; - contextInputStream.mark(readlimit); - } - - @Override - public synchronized void reset() throws IOException { - this.length = 0; - readAndCheck(mark); - contextInputStream.reset(); - } + final Object entity = context.proceed(); + context.setInputStream(contextInputStream); - @Override - public boolean markSupported() { - return contextInputStream.markSupported(); - } + return entity; + } - private void readAndCheck(final long read) { - this.length += read; - - if (this.length > declaredContentLength) { - try { - this.close(); - } catch (IOException e) { - logger.error("Error while closing the input stream", e); - } - throw new WebApplicationException( - Response.status(Response.Status.LENGTH_REQUIRED) - .entity("Incorrect content-length provided.") - .build() - ); + public static final class ContentSizeLimitInputStream extends InputStream { + private long length = 0; + private int mark = 0; + + private final long declaredContentLength; + private final int maxSize; + + private final InputStream contextInputStream; + + public ContentSizeLimitInputStream(InputStream contextInputStream, long declaredContentLength, int maxSize) { + this.contextInputStream = contextInputStream; + this.declaredContentLength = declaredContentLength; + this.maxSize = maxSize; + } + + @Override + public int read() throws IOException { + final int read = contextInputStream.read(); + readAndCheck(read != -1 ? 1 : 0); + return read; + } + + @Override + public int read(final byte[] b) throws IOException { + final int read = contextInputStream.read(b); + readAndCheck(read != -1 ? read : 0); + return read; + } + + @Override + public int read(final byte[] b, final int off, final int len) throws IOException { + final int read = contextInputStream.read(b, off, len); + readAndCheck(read != -1 ? read : 0); + return read; + } + + @Override + public long skip(final long n) throws IOException { + final long skip = contextInputStream.skip(n); + readAndCheck(skip != -1 ? skip : 0); + return skip; + } + + @Override + public int available() throws IOException { + return contextInputStream.available(); + } + + @Override + public void close() throws IOException { + contextInputStream.close(); + } + + @Override + public synchronized void mark(final int readlimit) { + mark += readlimit; + contextInputStream.mark(readlimit); + } + + @Override + public synchronized void reset() throws IOException { + this.length = 0; + readAndCheck(mark); + contextInputStream.reset(); + } + + @Override + public boolean markSupported() { + return contextInputStream.markSupported(); + } + + private void readAndCheck(final long read) { + this.length += read; + + if (this.length > declaredContentLength) { + try { + this.close(); + } catch (IOException e) { + logger.error("Error while closing the input stream", e); } - if (this.length > maxSize) { - try { - this.close(); - } catch (IOException e) { - logger.error("Error while closing the input stream", e); - } - throw new WebApplicationException( - Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) - .entity("Content size limit exceeded.") - .build() - ); + throw new WebApplicationException( + Response.status(Response.Status.LENGTH_REQUIRED) + .entity("Incorrect content-length provided.") + .build() + ); + } + if (this.length > maxSize) { + try { + this.close(); + } catch (IOException e) { + logger.error("Error while closing the input stream", e); } + throw new WebApplicationException( + Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) + .entity("Content size limit exceeded.") + .build() + ); } - }); - - final Object entity = context.proceed(); - context.setInputStream(contextInputStream); - - return entity; + } } } } diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java new file mode 100644 index 0000000..ad48e73 --- /dev/null +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java @@ -0,0 +1,11 @@ +package com.coreoz.plume.jersey; + +import com.google.inject.AbstractModule; + +public class WebJerseyTestModule extends AbstractModule { + + @Override + protected void configure() { + } + +} diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java new file mode 100644 index 0000000..89608c4 --- /dev/null +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java @@ -0,0 +1,101 @@ +package com.coreoz.plume.jersey.control; + +import org.assertj.core.api.Assertions; +import org.junit.Test; +import org.junit.After; +import org.junit.runner.RunWith; + +import java.io.IOException; + +import javax.ws.rs.WebApplicationException; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.io.ByteArrayInputStream; + +import com.carlosbecker.guice.GuiceModules; +import com.carlosbecker.guice.GuiceTestRunner; +import com.coreoz.plume.jersey.WebJerseyTestModule; +import com.coreoz.plume.jersey.security.control.ContentControlFeature.ContentSizeLimitInterceptor.ContentSizeLimitInputStream; + +@RunWith(GuiceTestRunner.class) +@GuiceModules(WebJerseyTestModule.class) +public class ContentSizeLimitTest { + + private static final int LIMIT = 10; + private ByteArrayInputStream byteArrayInputStream; + private ContentSizeLimitInputStream contentSizeLimitInputStream; + + @After + public void tearDown() throws IOException { + if (contentSizeLimitInputStream != null) { + contentSizeLimitInputStream.close(); + } + } + + @Test + public void test_read_within_limit() throws IOException { + byte[] data = "12345".getBytes(); + byteArrayInputStream = new ByteArrayInputStream(data); + contentSizeLimitInputStream = new ContentSizeLimitInputStream(byteArrayInputStream, data.length, LIMIT); + + byte[] buffer = new byte[data.length]; + int bytesRead = contentSizeLimitInputStream.read(buffer); + + assertEquals(data.length, bytesRead); + assertArrayEquals(data, buffer); + } + + @Test + public void test_read_beyond_limit() { + byte[] data = "12345678901".getBytes(); // 11 bytes, 1 byte over the limit + byteArrayInputStream = new ByteArrayInputStream(data); + contentSizeLimitInputStream = new ContentSizeLimitInputStream(byteArrayInputStream, data.length, LIMIT); + + byte[] buffer = new byte[data.length]; + assertThrows(WebApplicationException.class, () -> { + contentSizeLimitInputStream.read(buffer); + }); + } + + @Test + public void test_read_exactly_at_limit() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + contentSizeLimitInputStream = new ContentSizeLimitInputStream(byteArrayInputStream, data.length, LIMIT); + + byte[] buffer = new byte[data.length]; + int bytesRead = contentSizeLimitInputStream.read(buffer); + + assertEquals(data.length, bytesRead); + assertArrayEquals(data, buffer); + } + + @Test + public void test_read_from_empty_stream() throws IOException { + byte[] data = "".getBytes(); + byteArrayInputStream = new ByteArrayInputStream(data); + contentSizeLimitInputStream = new ContentSizeLimitInputStream(byteArrayInputStream, data.length, LIMIT); + + byte[] buffer = new byte[10]; + int bytesRead = contentSizeLimitInputStream.read(buffer); + + assertEquals(-1, bytesRead); + } + + @Test + public void test_incorrect_content_length() { + byte[] data = "12345".getBytes(); + byteArrayInputStream = new ByteArrayInputStream(data); + contentSizeLimitInputStream = new ContentSizeLimitInputStream(byteArrayInputStream, 3, LIMIT); + + byte[] buffer = new byte[data.length]; + + assertThrows(WebApplicationException.class, () -> { + contentSizeLimitInputStream.read(buffer); + }); + } + +} From 8f77c6dea1d514dcfb56353a273522938ae87adb Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Thu, 19 Sep 2024 15:20:00 +0200 Subject: [PATCH 3/8] Add readme doc about the ContentControlFeature --- plume-web-jersey/README.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/plume-web-jersey/README.md b/plume-web-jersey/README.md index d22c9f3..07dd03d 100644 --- a/plume-web-jersey/README.md +++ b/plume-web-jersey/README.md @@ -27,6 +27,34 @@ To use it, register this feature in Jersey: `resourceConfig.register(RequireExpl Any custom annotation can be added (as long as the corresponding Jersey access control feature is configured...). In a doubt to configure the Jersey access control feature, see as an example the existing class `PermissionFeature` that checks the `RestrictTo` annotation access control. +Content size limit +------------------ +In order to protect the backend against attack that would send huge content, it is possible to limit the size of the content that can be sent to the backend. + +To do so, register the `ContentControlFeature` in Jersey: `resourceConfig.register(ContentControlFeature.class);` +By default the content size of body is limited to 500 KB. This limit can be override for the whole api by using the `ContentControlFeatureFactory` to specify your own limit. + +Usage example: +```java +resourceConfig.register(new AbstractBinder() { + @Override + protected void configure() { + bindFactory(new ContentControlFeatureFactory(1000 * 1024 /* 1MB */)).to(ContentControlFeature.class); + } +}); +``` + +You can also override only a specific endpoint by using the `@ContentSizeLimit` annotation: +```java +@POST + @Path("/test") + @Operation(description = "Example web-service") + @ContentSizeLimit(1024 * 1000 * 5) // 5MB + public void test(Test test) { + logger.info("Test: {}", test.getName()); + } +``` + Data validation --------------- To validate web-service input data, an easy solution is to use `WsException`: From d922c438a7701bbf369e0e036ab06bf96a0a774d Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Thu, 19 Sep 2024 15:39:50 +0200 Subject: [PATCH 4/8] Add a fail fast for the content control feature if the content length header is already higher than the limit --- .../control/ContentControlFeature.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index fa1203a..615a7fa 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -9,10 +9,12 @@ import jakarta.ws.rs.container.DynamicFeature; import jakarta.ws.rs.container.ResourceInfo; import jakarta.ws.rs.core.FeatureContext; +import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ReaderInterceptor; import jakarta.ws.rs.ext.ReaderInterceptorContext; +import org.glassfish.grizzly.http.HttpHeader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,11 +56,27 @@ public ContentSizeLimitInterceptor(int maxSize) { // https://stackoverflow.com/questions/24516444/best-way-to-make-jersey-2-x-refuse-requests-with-incorrect-content-length @Override public Object aroundReadFrom(ReaderInterceptorContext context) throws IOException { - final InputStream contextInputStream = context.getInputStream(); + try { + final Integer headerContentLength = Integer.parseInt(context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH)); + if (headerContentLength > maxSize) { + throw new WebApplicationException( + Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) + .entity("Content size limit exceeded.") + .build() + ); + } - context.setInputStream(new SizeLimitingInputStream(contextInputStream, maxSize)); + final InputStream contextInputStream = context.getInputStream(); + context.setInputStream(new SizeLimitingInputStream(contextInputStream, headerContentLength)); - return context.proceed(); + return context.proceed(); + } catch (NumberFormatException e) { + throw new WebApplicationException( + Response.status(Response.Status.LENGTH_REQUIRED) + .entity("Content-Length header is missing or invalid.") + .build() + ); + } } public static final class SizeLimitingInputStream extends InputStream { From ec1660f39aaada4e24430b3c8180c1644d29993c Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Thu, 19 Sep 2024 17:20:51 +0200 Subject: [PATCH 5/8] add unit test for jersey content size limit --- plume-conf/pom.xml | 2 +- plume-web-jersey/pom.xml | 12 +++ .../control/ContentControlFeature.java | 30 +++--- .../plume/jersey/WebJerseyTestModule.java | 11 --- .../jersey/control/ContentSizeLimitTest.java | 98 +++++++------------ .../control/SizeLimitingInputStreamTest.java | 81 +++++++++++++++ .../control/TestContentSizeResource.java | 27 +++++ 7 files changed, 172 insertions(+), 89 deletions(-) delete mode 100644 plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java create mode 100644 plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java create mode 100644 plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java diff --git a/plume-conf/pom.xml b/plume-conf/pom.xml index cfe7154..d231b2e 100644 --- a/plume-conf/pom.xml +++ b/plume-conf/pom.xml @@ -47,7 +47,7 @@ - + diff --git a/plume-web-jersey/pom.xml b/plume-web-jersey/pom.xml index 4284064..4491317 100644 --- a/plume-web-jersey/pom.xml +++ b/plume-web-jersey/pom.xml @@ -121,6 +121,18 @@ assertj-core test + + org.glassfish.jersey.test-framework + jersey-test-framework-core + 3.1.1 + test + + + org.glassfish.jersey.test-framework.providers + jersey-test-framework-provider-grizzly2 + 3.1.1 + test + diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index 615a7fa..cf7aafc 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -14,7 +14,6 @@ import jakarta.ws.rs.ext.ReaderInterceptor; import jakarta.ws.rs.ext.ReaderInterceptorContext; -import org.glassfish.grizzly.http.HttpHeader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,7 +21,7 @@ public class ContentControlFeature implements DynamicFeature { private static final Logger logger = LoggerFactory.getLogger(ContentControlFeature.class); - private static final int DEFAULT_MAX_SIZE = 500 * 1024; // 500 KB + public static final int DEFAULT_MAX_SIZE = 500 * 1024; // 500 KB private final Integer maxSize; public ContentControlFeature(int maxSize) { @@ -56,20 +55,9 @@ public ContentSizeLimitInterceptor(int maxSize) { // https://stackoverflow.com/questions/24516444/best-way-to-make-jersey-2-x-refuse-requests-with-incorrect-content-length @Override public Object aroundReadFrom(ReaderInterceptorContext context) throws IOException { + Integer headerContentLength; try { - final Integer headerContentLength = Integer.parseInt(context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH)); - if (headerContentLength > maxSize) { - throw new WebApplicationException( - Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) - .entity("Content size limit exceeded.") - .build() - ); - } - - final InputStream contextInputStream = context.getInputStream(); - context.setInputStream(new SizeLimitingInputStream(contextInputStream, headerContentLength)); - - return context.proceed(); + headerContentLength = Integer.parseInt(context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH)); } catch (NumberFormatException e) { throw new WebApplicationException( Response.status(Response.Status.LENGTH_REQUIRED) @@ -77,6 +65,18 @@ public Object aroundReadFrom(ReaderInterceptorContext context) throws IOExceptio .build() ); } + if (headerContentLength > maxSize) { + throw new WebApplicationException( + Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE) + .entity("Content size limit exceeded.") + .build() + ); + } + + final InputStream contextInputStream = context.getInputStream(); + context.setInputStream(new SizeLimitingInputStream(contextInputStream, headerContentLength)); + + return context.proceed(); } public static final class SizeLimitingInputStream extends InputStream { diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java deleted file mode 100644 index ad48e73..0000000 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/WebJerseyTestModule.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.coreoz.plume.jersey; - -import com.google.inject.AbstractModule; - -public class WebJerseyTestModule extends AbstractModule { - - @Override - protected void configure() { - } - -} diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java index 219dc53..c98db3d 100644 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java @@ -1,88 +1,62 @@ package com.coreoz.plume.jersey.control; -import org.assertj.core.api.Assertions; import org.junit.Test; -import org.junit.After; -import org.junit.runner.RunWith; - -import java.io.IOException; import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.client.Entity; +import jakarta.ws.rs.core.Application; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.client.Invocation.Builder; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; - -import java.io.ByteArrayInputStream; +import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.JerseyTest; -import com.carlosbecker.guice.GuiceModules; -import com.carlosbecker.guice.GuiceTestRunner; -import com.coreoz.plume.jersey.WebJerseyTestModule; -import com.coreoz.plume.jersey.security.control.ContentControlFeature.ContentSizeLimitInterceptor.SizeLimitingInputStream; +import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; -@RunWith(GuiceTestRunner.class) -@GuiceModules(WebJerseyTestModule.class) -public class ContentSizeLimitTest { +import com.coreoz.plume.jersey.security.control.ContentControlFeature; - private static final int LIMIT = 10; - private ByteArrayInputStream byteArrayInputStream; - private SizeLimitingInputStream sizeLimitingInputStream; +public class ContentSizeLimitTest extends JerseyTest { - @After - public void tearDown() throws IOException { - if (sizeLimitingInputStream != null) { - sizeLimitingInputStream.close(); - } + @Override + protected Application configure() { + ResourceConfig config = new ResourceConfig(TestContentSizeResource.class); + config.register(ContentControlFeature.class); + return config; } @Test - public void test_read_within_limit() throws IOException { + public void checkContentSize_withBody_whenWithinDefaultLimit_shouldReturn200() { byte[] data = "12345".getBytes(); - byteArrayInputStream = new ByteArrayInputStream(data); - sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); - - byte[] buffer = new byte[data.length]; - int bytesRead = sizeLimitingInputStream.read(buffer); - - assertEquals(data.length, bytesRead); - assertArrayEquals(data, buffer); + Response response = target("/test/upload-default").request().post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)); + assertEquals(200, response.getStatus()); } @Test - public void test_read_beyond_limit() { - byte[] data = "12345678901".getBytes(); // 11 bytes, 1 byte over the limit - byteArrayInputStream = new ByteArrayInputStream(data); - sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); - - byte[] buffer = new byte[data.length]; - assertThrows(WebApplicationException.class, () -> { - sizeLimitingInputStream.read(buffer); - }); + public void checkContentSize_withBody_whenBeyondDefaultLimit_shouldThrow() { + // Generate a byte array of ContentControlFeature.DEFAULT_MAX_SIZE + 1 + byte[] data = new byte[ContentControlFeature.DEFAULT_MAX_SIZE + 1]; + Builder request = target("/test/upload-default").request(); + Entity entity = Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM); + assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus()); } @Test - public void test_read_exactly_at_limit() throws IOException { - byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit - byteArrayInputStream = new ByteArrayInputStream(data); - sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); - - byte[] buffer = new byte[data.length]; - int bytesRead = sizeLimitingInputStream.read(buffer); - - assertEquals(data.length, bytesRead); - assertArrayEquals(data, buffer); + public void checkContentSize_withBody_whenWithinCustomLimit_shouldReturn200() { + byte[] data = "12345".getBytes(); + Response response = target("/test/upload-custom").request().post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)); + assertEquals(200, response.getStatus()); } @Test - public void test_read_from_empty_stream() throws IOException { - byte[] data = "".getBytes(); - byteArrayInputStream = new ByteArrayInputStream(data); - sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); - - byte[] buffer = new byte[10]; - int bytesRead = sizeLimitingInputStream.read(buffer); - - assertEquals(-1, bytesRead); + public void checkContentSize_withBody_whenBeyondCustomLimit_shouldThrow() { + // Generate a byte array of CUSTOM_MAX_SIZE + 1 + byte[] data = new byte[TestContentSizeResource.CUSTOM_MAX_SIZE + 1]; + Builder request = target("/test/upload-custom").request(); + Entity entity = Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM); + assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus()); } } diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java new file mode 100644 index 0000000..5014776 --- /dev/null +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java @@ -0,0 +1,81 @@ +package com.coreoz.plume.jersey.control; + +import org.junit.Test; +import org.junit.After; + +import java.io.IOException; + +import jakarta.ws.rs.WebApplicationException; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.io.ByteArrayInputStream; + +import com.coreoz.plume.jersey.security.control.ContentControlFeature.ContentSizeLimitInterceptor.SizeLimitingInputStream; + +public class SizeLimitingInputStreamTest { + + private static final int LIMIT = 10; + private ByteArrayInputStream byteArrayInputStream; + private SizeLimitingInputStream sizeLimitingInputStream; + + @After + public void tearDown() throws IOException { + if (sizeLimitingInputStream != null) { + sizeLimitingInputStream.close(); + } + } + + @Test + public void testRead_whenWithinLimit_shouldSuccess() throws IOException { + byte[] data = "12345".getBytes(); + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + + byte[] buffer = new byte[data.length]; + int bytesRead = sizeLimitingInputStream.read(buffer); + + assertEquals(data.length, bytesRead); + assertArrayEquals(data, buffer); + } + + @Test + public void testRead_whenBeyondLimit_shouldThrow() { + byte[] data = "12345678901".getBytes(); // 11 bytes, 1 byte over the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + + byte[] buffer = new byte[data.length]; + assertThrows(WebApplicationException.class, () -> { + sizeLimitingInputStream.read(buffer); + }); + } + + @Test + public void testRead_whenExactLimit_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + + byte[] buffer = new byte[data.length]; + int bytesRead = sizeLimitingInputStream.read(buffer); + + assertEquals(data.length, bytesRead); + assertArrayEquals(data, buffer); + } + + @Test + public void testRead_whenEmpty_shouldSuccessWithoutReading() throws IOException { + byte[] data = "".getBytes(); + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + + byte[] buffer = new byte[10]; + int bytesRead = sizeLimitingInputStream.read(buffer); + + assertEquals(-1, bytesRead); + } + +} diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java new file mode 100644 index 0000000..b2a46c9 --- /dev/null +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java @@ -0,0 +1,27 @@ +package com.coreoz.plume.jersey.control; + +import com.coreoz.plume.jersey.security.control.ContentSizeLimit; + +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.core.Response; + +@Path("/test") +public class TestContentSizeResource { + + static final int CUSTOM_MAX_SIZE = 10; + + @POST + @Path("/upload-default") + public Response uploadDefaultLimit(byte[] data) { + return Response.ok("Upload successful").build(); + } + + @POST + @Path("/upload-custom") + @ContentSizeLimit(CUSTOM_MAX_SIZE) + public Response uploadCustomLimit(byte[] data) { + return Response.ok("Upload successful").build(); + } + +} From f6a9e0f09ea5d66ec3463c827e17bd9b2262892f Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Thu, 19 Sep 2024 18:35:45 +0200 Subject: [PATCH 6/8] Add unit test for sizelimitinginputstream for more coverage --- .../control/ContentControlFeature.java | 7 +++ .../jersey/control/ContentSizeLimitTest.java | 25 ++++++++--- .../control/SizeLimitingInputStreamTest.java | 45 +++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index cf7aafc..6776cf8 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -32,6 +32,13 @@ public ContentControlFeature() { this.maxSize = DEFAULT_MAX_SIZE; } + public Integer getContentSizeLimit() { + if (maxSize == null) { + return DEFAULT_MAX_SIZE; + } + return maxSize; + } + @Override public void configure(ResourceInfo resourceInfo, FeatureContext context) { addContentSizeFilter(resourceInfo.getResourceMethod(), context); diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java index c98db3d..9f60b69 100644 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java @@ -2,21 +2,20 @@ import org.junit.Test; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.core.Application; +import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.client.Invocation.Builder; -import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; import static org.junit.Assert.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import com.coreoz.plume.jersey.security.control.ContentControlFeature; +import com.coreoz.plume.jersey.security.control.ContentControlFeatureFactory; public class ContentSizeLimitTest extends JerseyTest { @@ -35,7 +34,7 @@ public void checkContentSize_withBody_whenWithinDefaultLimit_shouldReturn200() { } @Test - public void checkContentSize_withBody_whenBeyondDefaultLimit_shouldThrow() { + public void checkContentSize_withBody_whenBeyondDefaultLimit_shouldReturn413() { // Generate a byte array of ContentControlFeature.DEFAULT_MAX_SIZE + 1 byte[] data = new byte[ContentControlFeature.DEFAULT_MAX_SIZE + 1]; Builder request = target("/test/upload-default").request(); @@ -43,6 +42,14 @@ public void checkContentSize_withBody_whenBeyondDefaultLimit_shouldThrow() { assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus()); } + @Test + public void checkContentSize_withBody_whenContentLengthIsWrong_shouldReturn411() { + // Generate a byte array of ContentControlFeature.DEFAULT_MAX_SIZE + 1 + Builder request = target("/test/upload-default").request(); + request.header(HttpHeaders.CONTENT_LENGTH, null); + assertEquals(Response.Status.LENGTH_REQUIRED.getStatusCode(), request.post(null).getStatus()); + } + @Test public void checkContentSize_withBody_whenWithinCustomLimit_shouldReturn200() { byte[] data = "12345".getBytes(); @@ -51,7 +58,7 @@ public void checkContentSize_withBody_whenWithinCustomLimit_shouldReturn200() { } @Test - public void checkContentSize_withBody_whenBeyondCustomLimit_shouldThrow() { + public void checkContentSize_withBody_whenBeyondCustomLimit_shouldReturn413() { // Generate a byte array of CUSTOM_MAX_SIZE + 1 byte[] data = new byte[TestContentSizeResource.CUSTOM_MAX_SIZE + 1]; Builder request = target("/test/upload-custom").request(); @@ -59,4 +66,12 @@ public void checkContentSize_withBody_whenBeyondCustomLimit_shouldThrow() { assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus()); } + @Test + public void checkMaxSize_whenCustomControlFeature_shouldSuccess() { + // Custom max size + Integer customMaxSize = 300; + ContentControlFeatureFactory contentControlFeatureFactory = new ContentControlFeatureFactory(customMaxSize); + ContentControlFeature contentControlFeature = contentControlFeatureFactory.provide(); + assertEquals(customMaxSize, contentControlFeature.getContentSizeLimit()); + } } diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java index 5014776..81175a1 100644 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/SizeLimitingInputStreamTest.java @@ -78,4 +78,49 @@ public void testRead_whenEmpty_shouldSuccessWithoutReading() throws IOException assertEquals(-1, bytesRead); } + @Test + public void testRead_withoutOffsetAndLength_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + int result = sizeLimitingInputStream.read(data, 0, 5); + assertEquals(5, result); + } + + @Test + public void testRead_withoutOffsetAndWithoutLength_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + int result = sizeLimitingInputStream.read(); + assertEquals('1', result); + } + + @Test + public void testRead_withOffsetAndLength_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + int result = sizeLimitingInputStream.read(data, 3, 5); + assertEquals(5, result); + } + + @Test + public void testSkip_withinLength_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + long result = sizeLimitingInputStream.skip(3); + assertEquals(3, result); + } + + @Test + public void testSkip_beyondLength_shouldSuccess() throws IOException { + byte[] data = "1234567890".getBytes(); // 10 bytes, exactly at the limit + byteArrayInputStream = new ByteArrayInputStream(data); + sizeLimitingInputStream = new SizeLimitingInputStream(byteArrayInputStream, LIMIT); + long result = sizeLimitingInputStream.skip(11); + assertEquals(10, result); + } + } From 295ce44d52abb94685fc224461b66bc13bcce8d3 Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Fri, 20 Sep 2024 17:36:21 +0200 Subject: [PATCH 7/8] Add a unit test for the content control feature with GET request --- .../control/ContentControlFeature.java | 14 +++---------- .../jersey/control/ContentSizeLimitTest.java | 21 +++++++++++++++---- .../control/TestContentSizeResource.java | 14 +++++++++++++ 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index 6776cf8..902620c 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -4,7 +4,6 @@ import java.io.InputStream; import java.lang.reflect.AnnotatedElement; -import jakarta.inject.Singleton; import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.container.DynamicFeature; import jakarta.ws.rs.container.ResourceInfo; @@ -13,14 +12,11 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ReaderInterceptor; import jakarta.ws.rs.ext.ReaderInterceptorContext; +import lombok.extern.slf4j.Slf4j; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Singleton +@Slf4j public class ContentControlFeature implements DynamicFeature { - private static final Logger logger = LoggerFactory.getLogger(ContentControlFeature.class); public static final int DEFAULT_MAX_SIZE = 500 * 1024; // 500 KB private final Integer maxSize; @@ -66,11 +62,7 @@ public Object aroundReadFrom(ReaderInterceptorContext context) throws IOExceptio try { headerContentLength = Integer.parseInt(context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH)); } catch (NumberFormatException e) { - throw new WebApplicationException( - Response.status(Response.Status.LENGTH_REQUIRED) - .entity("Content-Length header is missing or invalid.") - .build() - ); + headerContentLength = maxSize; // default value for GET or chunked body } if (headerContentLength > maxSize) { throw new WebApplicationException( diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java index 9f60b69..f23232f 100644 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java @@ -30,7 +30,7 @@ protected Application configure() { public void checkContentSize_withBody_whenWithinDefaultLimit_shouldReturn200() { byte[] data = "12345".getBytes(); Response response = target("/test/upload-default").request().post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)); - assertEquals(200, response.getStatus()); + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); } @Test @@ -43,18 +43,25 @@ public void checkContentSize_withBody_whenBeyondDefaultLimit_shouldReturn413() { } @Test - public void checkContentSize_withBody_whenContentLengthIsWrong_shouldReturn411() { + public void checkContentSize_withBody_whenContentLengthIsWrong_shouldReturn413() { // Generate a byte array of ContentControlFeature.DEFAULT_MAX_SIZE + 1 + byte[] data = new byte[ContentControlFeature.DEFAULT_MAX_SIZE + 1]; Builder request = target("/test/upload-default").request(); request.header(HttpHeaders.CONTENT_LENGTH, null); - assertEquals(Response.Status.LENGTH_REQUIRED.getStatusCode(), request.post(null).getStatus()); + assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)).getStatus()); + } + + @Test + public void checkContentSize_withoutBody_whenDefaultLimit_shouldReturn200() { + Builder request = target("/test/upload-default").request(); + assertEquals(Response.Status.OK.getStatusCode(), request.get().getStatus()); } @Test public void checkContentSize_withBody_whenWithinCustomLimit_shouldReturn200() { byte[] data = "12345".getBytes(); Response response = target("/test/upload-custom").request().post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)); - assertEquals(200, response.getStatus()); + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); } @Test @@ -66,6 +73,12 @@ public void checkContentSize_withBody_whenBeyondCustomLimit_shouldReturn413() { assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus()); } + @Test + public void checkContentSize_withoutBody_whenCustomLimit_shouldReturn200() { + Builder request = target("/test/upload-custom").request(); + assertEquals(Response.Status.OK.getStatusCode(), request.get().getStatus()); + } + @Test public void checkMaxSize_whenCustomControlFeature_shouldSuccess() { // Custom max size diff --git a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java index b2a46c9..e6e6b59 100644 --- a/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java +++ b/plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/TestContentSizeResource.java @@ -2,6 +2,7 @@ import com.coreoz.plume.jersey.security.control.ContentSizeLimit; +import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.core.Response; @@ -17,6 +18,12 @@ public Response uploadDefaultLimit(byte[] data) { return Response.ok("Upload successful").build(); } + @GET + @Path("/upload-default") + public Response getDefaultLimit(byte[] data) { + return Response.ok("get successful").build(); + } + @POST @Path("/upload-custom") @ContentSizeLimit(CUSTOM_MAX_SIZE) @@ -24,4 +31,11 @@ public Response uploadCustomLimit(byte[] data) { return Response.ok("Upload successful").build(); } + @GET + @Path("/upload-custom") + @ContentSizeLimit(CUSTOM_MAX_SIZE) + public Response getCustomLimit(byte[] data) { + return Response.ok("get successful").build(); + } + } From 0c05d1d7f054f3d1755c389d9cea075ba3eb4ef6 Mon Sep 17 00:00:00 2001 From: Benoit Vasseur Date: Mon, 23 Sep 2024 18:05:27 +0200 Subject: [PATCH 8/8] =?UTF-8?q?Refacto=20ContentControlFeature=20pour=20ne?= =?UTF-8?q?=20pas=20g=C3=A9n=C3=A9rer=20une=20exception=20=C3=A0=20chaque?= =?UTF-8?q?=20GET?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../security/control/ContentControlFeature.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java index 902620c..f70c740 100644 --- a/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java +++ b/plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java @@ -58,12 +58,16 @@ public ContentSizeLimitInterceptor(int maxSize) { // https://stackoverflow.com/questions/24516444/best-way-to-make-jersey-2-x-refuse-requests-with-incorrect-content-length @Override public Object aroundReadFrom(ReaderInterceptorContext context) throws IOException { - Integer headerContentLength; - try { - headerContentLength = Integer.parseInt(context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH)); - } catch (NumberFormatException e) { - headerContentLength = maxSize; // default value for GET or chunked body + int headerContentLength = maxSize; // default value for GET or chunked body + String contentLengthHeader = context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH); + if (contentLengthHeader != null) { + try { + headerContentLength = Integer.parseInt(contentLengthHeader); + } catch (NumberFormatException e) { + logger.warn("Wrong content length header received: {}", contentLengthHeader); + } } + if (headerContentLength > maxSize) { throw new WebApplicationException( Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE)