Merge pull request #185 from swisspost/develop

PR for release

README.md

@@ -127,12 +127,17 @@ would lead to this result
 To use the StorageExpand feature you have to make a POST request to the desired collection to expand having the url parameter **storageExpand=true**. Also, you wil have
 to send the names of the sub resources in the body of the request. Using the example above, the request would look like this:
 
-**POST /yourStorageURL/collection** with the body:
+**POST /yourStorageURL/collection?storageExpand=true** with the body:
 ```json
 {
     "subResources" : ["resource1", "resource2", "resource3"]
 }
 ```
+The amount of sub resources that can be provided is defined in the configuration by the property _maxStorageExpandSubresources_.
+
+To override this for a single request, add the following request header with an appropriate value:
+> x-max-expand-resources: 1500
+
 
 ### Reject PUT requests on low memory (redis only)
 The redis storage provides a feature to reject PUT requests when the memory gets low. The information about the used memory is provided by the
@@ -201,6 +206,7 @@ The following configuration values are available:
 | storageAddress                          | common | resource-storage         | The eventbus address the mod listens to.                                                                                              |
 | editorConfig                            | common |                          | Additional configuration values for the editor                                                                                        |
 | confirmCollectionDelete                 | common | false                    | When set to _true_, an additional _recursive=true_ url parameter has to be set to delete collections                                  |
+| maxStorageExpandSubresources            | common | 1000                     | The amount of sub resources to expand. When limit exceeded, _413 Payload Too Large_ is returned                                       |
 | redisHost                               | redis  | localhost                | The host where redis is running on                                                                                                    |
 | redisPort                               | redis  | 6379                     | The port where redis is running on                                                                                                    |
 | redisReconnectAttempts                  | redis  | 0                        | The amount of reconnect attempts when connection to redis is lost. Use _-1_ for continuous reconnects or _0_ for no reconnects at all |

pom.xml

@@ -2,7 +2,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>org.swisspush</groupId>
     <artifactId>rest-storage</artifactId>
-    <version>3.1.3-SNAPSHOT</version>
+    <version>3.1.4-SNAPSHOT</version>
     <name>rest-storage</name>
     <description>
         Persistence for REST resources in the filesystem or a redis database

src/main/java/org/swisspush/reststorage/FileSystemRestStorageRunner.java

@@ -3,7 +3,6 @@
 import io.vertx.core.AsyncResult;
 import io.vertx.core.Vertx;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import static org.slf4j.LoggerFactory.getLogger;
 

src/main/java/org/swisspush/reststorage/MimeTypeResolver.java

@@ -1,7 +1,6 @@
 package org.swisspush.reststorage;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -14,7 +13,7 @@
 public class MimeTypeResolver {
 
     private static final Logger log = getLogger(MimeTypeResolver.class);
-    private Map<String, String> mimeTypes = new HashMap<>();
+    private final Map<String, String> mimeTypes = new HashMap<>();
 
     private final String defaultMimeType;
 

src/main/java/org/swisspush/reststorage/ModuleConfigurationAuthentication.java

@@ -9,7 +9,6 @@
 import io.vertx.ext.auth.authentication.Credentials;
 import io.vertx.ext.auth.authentication.UsernamePasswordCredentials;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.swisspush.reststorage.util.ModuleConfiguration;
 
 import java.util.Objects;

src/main/java/org/swisspush/reststorage/RedisRestStorageRunner.java

@@ -4,7 +4,6 @@
 import io.vertx.core.DeploymentOptions;
 import io.vertx.core.Vertx;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.swisspush.reststorage.util.ModuleConfiguration;
 
 import static org.slf4j.LoggerFactory.getLogger;

src/main/java/org/swisspush/reststorage/RestStorageHandler.java

@@ -16,7 +16,6 @@
 import org.slf4j.Logger;
 import org.swisspush.reststorage.util.*;
 
-import java.nio.charset.StandardCharsets;
 import java.text.DecimalFormat;
 import java.util.*;
 
@@ -42,6 +41,7 @@ public class RestStorageHandler implements Handler<HttpServerRequest> {
     private final boolean rejectStorageWriteOnLowMemory;
     private final boolean return200onDeleteNonExisting;
     private final DecimalFormat decimalFormat;
+    private final Integer maxStorageExpandSubresources;
 
     public RestStorageHandler(Vertx vertx, final Logger log, final Storage storage, final ModuleConfiguration config) {
         this.router = Router.router(vertx);
@@ -51,6 +51,7 @@ public RestStorageHandler(Vertx vertx, final Logger log, final Storage storage,
         this.confirmCollectionDelete = config.isConfirmCollectionDelete();
         this.rejectStorageWriteOnLowMemory = config.isRejectStorageWriteOnLowMemory();
         this.return200onDeleteNonExisting = config.isReturn200onDeleteNonExisting();
+        this.maxStorageExpandSubresources = config.getMaxStorageExpandSubresources();
 
         this.decimalFormat = new DecimalFormat();
         this.decimalFormat.setMaximumFractionDigits(1);
@@ -192,7 +193,7 @@ public void handle(Resource resource) {
 
                             StringBuilder body = new StringBuilder(1024);
                             String editor = null;
-                            if (editors.size() > 0) {
+                            if (!editors.isEmpty()) {
                                 editor = editors.values().iterator().next();
                             }
                             body.append("<!DOCTYPE html>\n");
@@ -277,12 +278,8 @@ public void handle(Resource resource) {
                                 documentResource.closeHandler.handle(null);
                                 rsp.end();
                             });
-                            documentResource.addErrorHandler(ex -> {
-                                log.error("TODO error handling", new Exception(ex));
-                            });
-                            documentResource.readStream.exceptionHandler((Handler<Throwable>) ex -> {
-                                log.error("TODO error handling", new Exception(ex));
-                            });
+                            documentResource.addErrorHandler(ex -> log.error("TODO error handling", new Exception(ex)));
+                            documentResource.readStream.exceptionHandler(ex -> log.error("TODO error handling", new Exception(ex)));
                             pump.start();
                         }
                     }
@@ -586,17 +583,35 @@ private void deleteResource(RoutingContext ctx) {
                 });
     }
 
+    private boolean checkMaxSubResourcesCount(HttpServerRequest request, int subResourcesArraySize) {
+        MultiMap headers = request.headers();
+
+        // get max expand value from request header or use the configuration value as fallback
+        Integer maxStorageExpand = getInteger(headers, MAX_EXPAND_RESOURCES_HEADER, maxStorageExpandSubresources);
+        if (maxStorageExpand < subResourcesArraySize) {
+            respondWith(request.response(), StatusCode.PAYLOAD_TOO_LARGE,
+                    "Resources provided: "+subResourcesArraySize+". Allowed are: " + maxStorageExpand);
+            return true;
+        }
+        return false;
+    }
+
     private void storageExpand(RoutingContext ctx) {
-        if (!containsParam(ctx.request().params(), STORAGE_EXPAND_PARAMETER)) {
-            respondWithNotAllowed(ctx.request());
+        HttpServerRequest request = ctx.request();
+        if (!containsParam(request.params(), STORAGE_EXPAND_PARAMETER)) {
+            respondWithNotAllowed(request);
         } else {
-            ctx.request().bodyHandler(bodyBuf -> {
+            request.bodyHandler(bodyBuf -> {
                 List<String> subResourceNames = new ArrayList<>();
                 try {
                     JsonObject body = new JsonObject(bodyBuf);
                     JsonArray subResourcesArray = body.getJsonArray("subResources");
                     if (subResourcesArray == null) {
-                        respondWithBadRequest(ctx.request(), "Bad Request: Expected array field 'subResources' with names of resources");
+                        respondWithBadRequest(request, "Bad Request: Expected array field 'subResources' with names of resources");
+                        return;
+                    }
+
+                    if (checkMaxSubResourcesCount(request, subResourcesArray.size())) {
                         return;
                     }
 
@@ -606,12 +621,12 @@ private void storageExpand(RoutingContext ctx) {
                     ResourceNameUtil.replaceColonsAndSemiColonsInList(subResourceNames);
                 } catch (RuntimeException ex) {
                     log.warn("KISS handler is not interested in error details. I'll report them here then.", ex);
-                    respondWithBadRequest(ctx.request(), "Bad Request: Unable to parse body of storageExpand POST request");
+                    respondWithBadRequest(request, "Bad Request: Unable to parse body of storageExpand POST request");
                     return;
                 }
 
-                final String path = cleanPath(ctx.request().path().substring(prefixFixed.length()));
-                final String etag = ctx.request().headers().get(IF_NONE_MATCH_HEADER.getName());
+                final String path = cleanPath(request.path().substring(prefixFixed.length()));
+                final String etag = request.headers().get(IF_NONE_MATCH_HEADER.getName());
                 storage.storageExpand(path, etag, subResourceNames, resource -> {
                     var rsp = ctx.response();
 
@@ -649,7 +664,7 @@ private void storageExpand(RoutingContext ctx) {
 
                     if (resource.exists) {
                         if (log.isTraceEnabled()) {
-                            log.trace("RestStorageHandler resource is a DocumentResource: {}", ctx.request().uri());
+                            log.trace("RestStorageHandler resource is a DocumentResource: {}", request.uri());
                         }
 
                         String mimeType = mimeTypeResolver.resolveMimeType(path);
@@ -669,7 +684,7 @@ private void storageExpand(RoutingContext ctx) {
 
                     } else {
                         if (log.isTraceEnabled()) {
-                            log.trace("RestStorageHandler Could not find resource: {}", ctx.request().uri());
+                            log.trace("RestStorageHandler Could not find resource: {}", request.uri());
                         }
                         rsp.setStatusCode(StatusCode.NOT_FOUND.getStatusCode());
                         rsp.setStatusMessage(StatusCode.NOT_FOUND.getStatusMessage());
@@ -730,14 +745,15 @@ private void respondWith(HttpServerResponse response, StatusCode statusCode, Str
         response.setStatusCode(statusCode.getStatusCode());
         response.setStatusMessage(statusCode.getStatusMessage());
         if (responseBody != null) {
+            response.putHeader(CONTENT_TYPE.getName(), "text/plain");
             response.end(responseBody);
         } else {
             response.end();
         }
     }
 
     private String collectionName(String path) {
-        if (path.equals("/") || path.equals("")) {
+        if (path.equals("/") || path.isEmpty()) {
             return "root";
         } else {
             return path.substring(path.lastIndexOf("/") + 1);

src/main/java/org/swisspush/reststorage/redis/RedisStorage.java

@@ -51,6 +51,8 @@
 import java.util.UUID;
 import java.util.stream.Stream;
 
+import static org.swisspush.reststorage.redis.RedisUtils.toPayload;
+
 public class RedisStorage implements Storage {
 
     private final Logger log = LoggerFactory.getLogger(RedisStorage.class);
@@ -815,7 +817,7 @@ private void handleJsonArrayValues(Response values, Handler<Resource> handler, b
                     }
                 }
             }
-            if (allowEmptyReturn && items.size() == 0) {
+            if (allowEmptyReturn && items.isEmpty()) {
                 notFound(handler);
             } else {
                 r.items = new ArrayList<>(items);
@@ -1293,46 +1295,4 @@ public void cleanup(Handler<DocumentResource> handler, String cleanupResourcesAm
     private boolean isEmpty(CharSequence cs) {
         return cs == null || cs.length() == 0;
     }
-
-    /**
-     * from https://github.com/vert-x3/vertx-redis-client/blob/3.9/src/main/java/io/vertx/redis/impl/RedisClientImpl.java#L94
-     *
-     * @param parameters
-     * @return
-     */
-    private static List<String> toPayload(Object... parameters) {
-        List<String> result = new ArrayList<>(parameters.length);
-
-        for (Object param : parameters) {
-            // unwrap
-            if (param instanceof JsonArray) {
-                param = ((JsonArray) param).getList();
-            }
-            // unwrap
-            if (param instanceof JsonObject) {
-                param = ((JsonObject) param).getMap();
-            }
-
-            if (param instanceof Collection) {
-                ((Collection) param).stream().filter(Objects::nonNull).forEach(o -> result.add(o.toString()));
-            } else if (param instanceof Map) {
-                for (Map.Entry<?, ?> pair : ((Map<?, ?>) param).entrySet()) {
-                    result.add(pair.getKey().toString());
-                    result.add(pair.getValue().toString());
-                }
-            } else if (param instanceof Stream) {
-                ((Stream) param).forEach(e -> {
-                    if (e instanceof Object[]) {
-                        Collections.addAll(result, (String[]) e);
-                    } else {
-                        result.add(e.toString());
-                    }
-                });
-            } else if (param != null) {
-                result.add(param.toString());
-            }
-        }
-        return result;
-    }
-
 }

src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java

@@ -16,6 +16,7 @@ public enum HttpRequestHeader {
     LOCK_EXPIRE_AFTER_HEADER("x-lock-expire-after"),
     EXPIRE_AFTER_HEADER("x-expire-after"),
     IMPORTANCE_LEVEL_HEADER("x-importance-level"),
+    MAX_EXPAND_RESOURCES_HEADER("x-max-expand-resources"),
     COMPRESS_HEADER("x-stored-compressed"),
     CONTENT_TYPE("Content-Type"),
     CONTENT_LENGTH("Content-Length");

src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java

@@ -67,6 +67,7 @@ public enum StorageType {
     private int maxRedisConnectionPoolSize = 24;
     private int maxQueueWaiting = 24;
     private int maxRedisWaitingHandlers = 2048;
+    private int maxStorageExpandSubresources = 1000;
 
 
     public ModuleConfiguration root(String root) {
@@ -280,6 +281,11 @@ public ModuleConfiguration return200onDeleteNonExisting(boolean deleteNonExistin
         return this;
     }
 
+    public ModuleConfiguration maxStorageExpandSubresources(int maxStorageExpandSubresources) {
+        this.maxStorageExpandSubresources = maxStorageExpandSubresources;
+        return this;
+    }
+
     public String getRoot() {
         return root;
     }
@@ -445,6 +451,10 @@ public int getMaxRedisWaitingHandlers() {
         return maxRedisWaitingHandlers;
     }
 
+    public int getMaxStorageExpandSubresources() {
+        return maxStorageExpandSubresources;
+    }
+
     public JsonObject asJsonObject() {
         return JsonObject.mapFrom(this);
     }

src/main/java/org/swisspush/reststorage/util/ResourcesUtils.java

@@ -16,7 +16,7 @@
  */
 public class ResourcesUtils {
 
-    private static Logger log = LoggerFactory.getLogger(ResourcesUtils.class);
+    private static final Logger log = LoggerFactory.getLogger(ResourcesUtils.class);
 
     private ResourcesUtils() {
         // prevent instantiation

src/main/java/org/swisspush/reststorage/util/StatusCode.java

@@ -13,6 +13,7 @@ public enum StatusCode {
     UNAUTHORIZED(401, "Unauthorized"),
     NOT_FOUND(404, "Not Found"),
     METHOD_NOT_ALLOWED(405, "Method Not Allowed"),
+    PAYLOAD_TOO_LARGE(413, "Payload Too Large"),
     INTERNAL_SERVER_ERROR(500, "Internal Server Error"),
     INSUFFICIENT_STORAGE(507, "Insufficient Storage"),
     CONFLICT(409, "Conflict");

src/main/resources/mime-types.properties

@@ -28,7 +28,7 @@ aso=application/vnd.accpac.simply.aso
 atc=application/vnd.acucorp
 atomcat=application/atomcat+xml
 atomsvc=application/atomsvc+xml
-atom, xml=application/atom+xml
+atomxml=application/atom+xml
 atx=application/vnd.antix.game-component
 au=audio/basic
 avi=video/x-msvideo

src/test/java/org/swisspush/reststorage/EtagIntegrationTest.java

@@ -26,7 +26,7 @@ public void testEtag(TestContext context) {
 
         String etag = get("resources/res1").getHeader(ETAG_HEADER);
         context.assertNotNull(etag, "Etag header should be available in response headers");
-        context.assertTrue(etag.length() > 0, "Etag header should not be empty");
+        context.assertFalse(etag.isEmpty(), "Etag header should not be empty");
 
         // get requests with no if-none-match header should result in statuscode 200
         when().get("resources/res1").then().assertThat()

src/test/java/org/swisspush/reststorage/FilesystemStorageTest.java

@@ -346,6 +346,6 @@ private void doAsserts() {
      *      check if such a directory exists or someone has access to it.
      */
     private static String createPseudoFileStorageRoot() {
-        return new File( "target/fileStorage-"+ UUID.randomUUID().toString() ).getAbsolutePath().replaceAll("\\\\","/"); // <-- Fix windows
+        return new File( "target/fileStorage-"+ UUID.randomUUID()).getAbsolutePath().replaceAll("\\\\","/"); // <-- Fix windows
     }
 }

src/test/java/org/swisspush/reststorage/RestStorageHandlerTest.java

@@ -312,7 +312,7 @@ public void notifiesResourceAboutExceptionsOnRequest(TestContext testContext) th
                 @Override
                 public void put(String path, String etag, boolean merge, long expire, String lockOwner, LockMode lockMode, long lockExpire, boolean storeCompressed, Handler<Resource> handler) {
                     final DocumentResource resource = new DocumentResource();
-                    resource.writeStream = new FailFastVertxWriteStream<Buffer>() {
+                    resource.writeStream = new FailFastVertxWriteStream<>() {
                         @Override
                         public Future<Void> write(Buffer t) {
                             log.debug("Somewhat irrelevant got written to the resource.");