Merge remote-tracking branch 'origin/master' into authorization-feature-and-bearer

Author: amanteaux

plume-framework-dependencies/pom.xml

@@ -31,15 +31,15 @@ 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.
+To do so, register the `ContentSizeLimitFeature` in Jersey: `resourceConfig.register(ContentSizeLimitFeature.class);`
+By default the content size of body is limited to 500 KB. This limit can be overridden for the whole api by using the `ContentSizeLimitFeatureFactory` 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);
+        bindFactory(new ContentSizeLimitFeatureFactory(1000 * 1024 /* 1MB */)).to(ContentSizeLimitFeature.class);
     }
 });
 ```

plume-web-jersey/README.md

@@ -12,6 +12,7 @@ public interface WsError {
 	WsError FIELD_REQUIRED = new WsErrorInternal("FIELD_REQUIRED");
 	WsError EMAIL_INVALID = new WsErrorInternal("EMAIL_INVALID");
 	WsError COLOR_INVALID = new WsErrorInternal("COLOR_INVALID");
+	WsError CONTENT_SIZE_LIMIT_EXCEEDED = new WsErrorInternal("CONTENT_SIZE_LIMIT_EXCEEDED");
 
 	/**
 	 * Returns the name of the error

plume-web-jersey/src/main/java/com/coreoz/plume/jersey/errors/WsError.java

@@ -1,4 +1,4 @@
-package com.coreoz.plume.jersey.security.control;
+package com.coreoz.plume.jersey.security.size;
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.ElementType.TYPE;

plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeatureFactory.java

@@ -1,37 +1,37 @@
-package com.coreoz.plume.jersey.security.control;
+package com.coreoz.plume.jersey.security.size;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.lang.reflect.AnnotatedElement;
-
-import jakarta.ws.rs.WebApplicationException;
+import com.coreoz.plume.jersey.errors.WsError;
+import jakarta.ws.rs.ClientErrorException;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
 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.MediaType;
 import jakarta.ws.rs.core.Response;
-import jakarta.ws.rs.ext.ReaderInterceptor;
-import jakarta.ws.rs.ext.ReaderInterceptorContext;
 import lombok.extern.slf4j.Slf4j;
 
-@Slf4j
-public class ContentControlFeature implements DynamicFeature {
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.AnnotatedElement;
 
+@Slf4j
+public class ContentSizeLimitFeature implements DynamicFeature {
     public static final int DEFAULT_MAX_SIZE = 500 * 1024; // 500 KB
+    // We use a string response directly because Jersey does not accept an objet here (it would return a 500 error)
+    static final String JSON_ENTITY_TOO_LARGE_ERROR = "{\"errorCode\":\""+WsError.CONTENT_SIZE_LIMIT_EXCEEDED.name()+"\",\"statusArguments\":[]}";
     private final Integer maxSize;
 
-    public ContentControlFeature(int maxSize) {
+    public ContentSizeLimitFeature(int maxSize) {
         this.maxSize = maxSize;
     }
 
-    public ContentControlFeature() {
+    public ContentSizeLimitFeature() {
         this.maxSize = DEFAULT_MAX_SIZE;
     }
 
     public Integer getContentSizeLimit() {
-        if (maxSize == null) {
-            return DEFAULT_MAX_SIZE;
-        }
         return maxSize;
     }
 
@@ -40,15 +40,14 @@ public void configure(ResourceInfo resourceInfo, FeatureContext context) {
         addContentSizeFilter(resourceInfo.getResourceMethod(), context);
     }
 
-    private void addContentSizeFilter(AnnotatedElement annotatedElement, FeatureContext methodResourcecontext) {
+    private void addContentSizeFilter(AnnotatedElement annotatedElement, FeatureContext methodResourceContext) {
         ContentSizeLimit contentSizeLimit = annotatedElement.getAnnotation(ContentSizeLimit.class);
-        methodResourcecontext.register(new ContentSizeLimitInterceptor(
-                contentSizeLimit != null ? contentSizeLimit.value() : maxSize
-            ));
+        methodResourceContext.register(new ContentSizeLimitInterceptor(
+            contentSizeLimit != null ? contentSizeLimit.value() : maxSize
+        ));
     }
 
-    public static class ContentSizeLimitInterceptor implements ReaderInterceptor {
-
+    public static class ContentSizeLimitInterceptor implements ContainerRequestFilter {
         private final int maxSize;
 
         public ContentSizeLimitInterceptor(int maxSize) {
@@ -57,7 +56,7 @@ 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 {
+        public void filter(ContainerRequestContext context) {
             int headerContentLength = maxSize; // default value for GET or chunked body
             String contentLengthHeader = context.getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH);
             if (contentLengthHeader != null) {
@@ -69,17 +68,12 @@ public Object aroundReadFrom(ReaderInterceptorContext context) throws IOExceptio
             }
 
             if (headerContentLength > maxSize) {
-                throw new WebApplicationException(
-                    Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE)
-                            .entity("Content size limit exceeded.")
-                            .build()
-                );
+                logger.warn("Client tried to send a request body that is too large: max allowed size={}, client request body size={}", maxSize, headerContentLength);
+                throw makeEntityTooLargeException();
             }
 
-            final InputStream contextInputStream = context.getInputStream();
-            context.setInputStream(new SizeLimitingInputStream(contextInputStream, headerContentLength));
-
-            return context.proceed();
+            final InputStream contextInputStream = context.getEntityStream();
+            context.setEntityStream(new SizeLimitingInputStream(contextInputStream, headerContentLength));
         }
 
         public static final class SizeLimitingInputStream extends InputStream {
@@ -119,7 +113,7 @@ public int read(final byte[] b, final int off, final int len) throws IOException
             @Override
             public long skip(final long n) throws IOException {
                 final long skip = delegateInputStream.skip(n);
-                readAndCheck(skip != -1 ? skip : 0);
+                readAndCheck(skip);
                 return skip;
             }
 
@@ -160,13 +154,20 @@ private void readAndCheck(final long read) {
                     } 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()
-                    );
+                    logger.warn("Client tried to send a request body that is too large while sending a content-length headers that fits the max allowed request body size");
+                    throw makeEntityTooLargeException();
                 }
             }
         }
+
+        private static RuntimeException makeEntityTooLargeException() {
+            return new ClientErrorException(Response
+                .status(Response.Status.REQUEST_ENTITY_TOO_LARGE)
+                .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON)
+                // We use a string response directly because Jersey does not accept an objet here (it would return a 500 error)
+                .entity(JSON_ENTITY_TOO_LARGE_ERROR)
+                .build()
+            );
+        }
     }
 }

plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentSizeLimit.java renamed to plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/size/ContentSizeLimit.java

@@ -0,0 +1,21 @@
+package com.coreoz.plume.jersey.security.size;
+
+import org.glassfish.hk2.api.Factory;
+
+public class ContentSizeLimitFeatureFactory implements Factory<ContentSizeLimitFeature> {
+    private final Integer maxSize;
+
+    public ContentSizeLimitFeatureFactory(int maxSize) {
+        this.maxSize = maxSize;
+    }
+
+    @Override
+    public ContentSizeLimitFeature provide() {
+        return new ContentSizeLimitFeature(maxSize);
+    }
+
+    @Override
+    public void dispose(ContentSizeLimitFeature instance) {
+        // unused
+    }
+}

plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/control/ContentControlFeature.java renamed to plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/size/ContentSizeLimitFeature.java

@@ -1,28 +1,28 @@
-package com.coreoz.plume.jersey.control;
-
-import org.junit.Test;
+package com.coreoz.plume.jersey.security.size;
 
+import com.coreoz.plume.jersey.errors.WsError;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import jakarta.ws.rs.client.Entity;
+import jakarta.ws.rs.client.Invocation.Builder;
 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 lombok.SneakyThrows;
+import org.assertj.core.api.Assertions;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
+import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
 
-import com.coreoz.plume.jersey.security.control.ContentControlFeature;
-import com.coreoz.plume.jersey.security.control.ContentControlFeatureFactory;
-
 public class ContentSizeLimitTest  extends JerseyTest {
 
     @Override
     protected Application configure() {
         ResourceConfig config = new ResourceConfig(TestContentSizeResource.class);
-        config.register(ContentControlFeature.class);
+        config.register(ContentSizeLimitFeature.class);
         return config;
     }
 
@@ -33,19 +33,26 @@ public void checkContentSize_withBody_whenWithinDefaultLimit_shouldReturn200() {
         assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
     }
 
+    @SneakyThrows
     @Test
     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];
+        byte[] data = new byte[ContentSizeLimitFeature.DEFAULT_MAX_SIZE + 1];
         Builder request = target("/test/upload-default").request();
         Entity<byte[]> entity = Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM);
-        assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(entity).getStatus());
+        Response response = request.post(entity);
+        assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), response.getStatus());
+        // make sure that a valid json error response is returned
+        assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getMediaType());
+        JsonNode responseBody = new ObjectMapper().readTree(response.readEntity(String.class));
+        Assertions.assertThat(responseBody).isNotNull();
+        Assertions.assertThat(responseBody.get("errorCode").asText()).isEqualTo(WsError.CONTENT_SIZE_LIMIT_EXCEEDED.name());
     }
 
     @Test
     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];
+        byte[] data = new byte[ContentSizeLimitFeature.DEFAULT_MAX_SIZE + 1];
         Builder request = target("/test/upload-default").request();
         request.header(HttpHeaders.CONTENT_LENGTH, null);
         assertEquals(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode(), request.post(Entity.entity(data, MediaType.APPLICATION_OCTET_STREAM)).getStatus());
@@ -83,8 +90,8 @@ public void checkContentSize_withoutBody_whenCustomLimit_shouldReturn200() {
     public void checkMaxSize_whenCustomControlFeature_shouldSuccess() {
         // Custom max size
         Integer customMaxSize = 300;
-        ContentControlFeatureFactory contentControlFeatureFactory = new ContentControlFeatureFactory(customMaxSize);
-        ContentControlFeature contentControlFeature = contentControlFeatureFactory.provide();
-        assertEquals(customMaxSize, contentControlFeature.getContentSizeLimit());
+        ContentSizeLimitFeatureFactory contentControlFeatureFactory = new ContentSizeLimitFeatureFactory(customMaxSize);
+        ContentSizeLimitFeature contentSizeLimitFeature = contentControlFeatureFactory.provide();
+        assertEquals(customMaxSize, contentSizeLimitFeature.getContentSizeLimit());
     }
 }

plume-web-jersey/src/main/java/com/coreoz/plume/jersey/security/size/ContentSizeLimitFeatureFactory.java

@@ -1,4 +1,4 @@
-package com.coreoz.plume.jersey.control;
+package com.coreoz.plume.jersey.security.size;
 
 import org.junit.Test;
 import org.junit.After;
@@ -13,7 +13,7 @@
 
 import java.io.ByteArrayInputStream;
 
-import com.coreoz.plume.jersey.security.control.ContentControlFeature.ContentSizeLimitInterceptor.SizeLimitingInputStream;
+import com.coreoz.plume.jersey.security.size.ContentSizeLimitFeature.ContentSizeLimitInterceptor.SizeLimitingInputStream;
 
 public class SizeLimitingInputStreamTest {
 

plume-web-jersey/src/test/java/com/coreoz/plume/jersey/control/ContentSizeLimitTest.java renamed to plume-web-jersey/src/test/java/com/coreoz/plume/jersey/security/size/ContentSizeLimitTest.java

@@ -1,6 +1,4 @@
-package com.coreoz.plume.jersey.control;
-
-import com.coreoz.plume.jersey.security.control.ContentSizeLimit;
+package com.coreoz.plume.jersey.security.size;
 
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.POST;