From 330952f3a2614972486a8939c42e07e29067fb79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Weber?= Date: Wed, 1 May 2024 11:10:09 +0200 Subject: [PATCH] #173 Add limit for StorageExpand feature --- README.md | 8 ++- .../reststorage/RestStorageHandler.java | 38 ++++++++--- .../reststorage/util/HttpRequestHeader.java | 1 + .../reststorage/util/ModuleConfiguration.java | 10 +++ .../reststorage/util/StatusCode.java | 1 + .../StorageExpandIntegrationTest.java | 68 +++++++++++++++++++ .../RedisStorageIntegrationTestCase.java | 1 + .../util/ModuleConfigurationTest.java | 9 +++ 8 files changed, 126 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9db9eaa..d860fff 100644 --- a/README.md +++ b/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 | diff --git a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java index a91ec1d..c271438 100644 --- a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java +++ b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java @@ -42,6 +42,7 @@ public class RestStorageHandler implements Handler { 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 +52,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); @@ -586,17 +588,35 @@ private void deleteResource(RoutingContext ctx) { }); } + private boolean checkMaxSubResourcesCount(HttpServerRequest request, int subResourcesArraySize) { + MultiMap headers = request.headers(); + + // get max expand value from request header of 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 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 +626,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 +669,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 +689,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()); diff --git a/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java b/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java index 60db461..b34287e 100644 --- a/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java +++ b/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"); diff --git a/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java b/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java index efaebd8..5e5604f 100644 --- a/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java +++ b/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); } diff --git a/src/main/java/org/swisspush/reststorage/util/StatusCode.java b/src/main/java/org/swisspush/reststorage/util/StatusCode.java index 3d5bc5c..74aaced 100644 --- a/src/main/java/org/swisspush/reststorage/util/StatusCode.java +++ b/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"); diff --git a/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java b/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java index 6c117ff..8c67927 100644 --- a/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java +++ b/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java @@ -18,6 +18,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.swisspush.reststorage.redis.RedisStorageIntegrationTestCase; +import org.swisspush.reststorage.util.HttpRequestHeader; import static io.restassured.RestAssured.*; import static org.hamcrest.Matchers.*; @@ -250,6 +251,73 @@ public void testSimpleWithEmptyResult(TestContext context) { async.complete(); } + @Test + public void testWithTooManySubResources(TestContext context) { + Async async = context.async(); + delete("/server/resources"); + + // by configuration, only 5 sub resources are allowed + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 6. Allowed are: 5")); + + // request header overwrites configuration + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 2) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 6. Allowed are: 2")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 8) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 8")); + + // invalid header value + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), "Not a number") + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 999999999999999999L) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 25.5) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + // no 413 Payload Too Large response when limit not exceeded + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 10) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(404); + + async.complete(); + } + @Test public void testSimpleWithResourcesAndCollections(TestContext context) { Async async = context.async(); diff --git a/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java b/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java index e9831e1..138bd6c 100644 --- a/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java +++ b/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java @@ -34,6 +34,7 @@ public void setUp(TestContext context) { ModuleConfiguration modConfig = new ModuleConfiguration() .storageType(ModuleConfiguration.StorageType.redis) .confirmCollectionDelete(true) + .maxStorageExpandSubresources(5) .storageAddress("rest-storage"); updateModuleConfiguration(modConfig); diff --git a/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java b/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java index b1be26f..125647d 100644 --- a/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java +++ b/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java @@ -60,6 +60,7 @@ public void testDefaultConfiguration(TestContext testContext) { testContext.assertEquals(config.getFreeMemoryCheckIntervalMs(), 60000L); testContext.assertFalse(config.isReturn200onDeleteNonExisting()); testContext.assertEquals(config.getMaxRedisWaitingHandlers(), 2048); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 1000); } @Test @@ -85,6 +86,7 @@ public void testOverrideConfiguration(TestContext testContext) { .redisPublishMetrcisAddress("metrics-eb-address") .redisPublishMetrcisPrefix("my-storage") .redisPublishMetrcisRefreshPeriodSec(20) + .maxStorageExpandSubresources(500) .return200onDeleteNonExisting(true); // go through JSON encode/decode @@ -127,6 +129,7 @@ public void testOverrideConfiguration(TestContext testContext) { testContext.assertEquals(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "my-storage"); testContext.assertEquals(config.getRedisPublishMetrcisRefreshPeriodSec(), 20); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 500); } @Test @@ -167,6 +170,7 @@ public void testGetDefaultAsJsonObject(TestContext testContext){ testContext.assertFalse(json.getBoolean("confirmCollectionDelete")); testContext.assertFalse(json.getBoolean("rejectStorageWriteOnLowMemory")); testContext.assertEquals(json.getLong("freeMemoryCheckIntervalMs"), 60000L); + testContext.assertEquals(json.getInteger("maxStorageExpandSubresources"), 1000); } @Test @@ -193,6 +197,7 @@ public void testGetOverriddenAsJsonObject(TestContext testContext){ .confirmCollectionDelete(true) .rejectStorageWriteOnLowMemory(true) .resourceCleanupIntervalSec(15) + .maxStorageExpandSubresources(500) .freeMemoryCheckIntervalMs(5000); JsonObject json = config.asJsonObject(); @@ -225,6 +230,7 @@ public void testGetOverriddenAsJsonObject(TestContext testContext){ testContext.assertEquals(json.getLong("freeMemoryCheckIntervalMs"), 5000L); testContext.assertEquals(json.getInteger("maxRedisWaitingHandlers"), 4096); testContext.assertEquals(json.getInteger("resourceCleanupIntervalSec"), 15); + testContext.assertEquals(json.getInteger("maxStorageExpandSubresources"), 500); testContext.assertNotNull(json.getJsonObject("editorConfig")); testContext.assertTrue(json.getJsonObject("editorConfig").containsKey("myKey")); @@ -275,6 +281,7 @@ public void testGetDefaultFromJsonObject(TestContext testContext){ testContext.assertNull(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "storage"); testContext.assertEquals(config.getRedisPublishMetrcisRefreshPeriodSec(), 10); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 1000); } @Test @@ -312,6 +319,7 @@ public void testGetOverriddenFromJsonObject(TestContext testContext){ json.put("redisPublishMetrcisAddress", "metrics-eb-address"); json.put("redisPublishMetrcisPrefix", "my-storage"); json.put("redisPublishMetrcisRefreshPeriodSec", 30); + json.put("maxStorageExpandSubresources", 250); ModuleConfiguration config = fromJsonObject(json); testContext.assertEquals(config.getRoot(), "newroot"); @@ -346,6 +354,7 @@ public void testGetOverriddenFromJsonObject(TestContext testContext){ testContext.assertTrue(config.isConfirmCollectionDelete()); testContext.assertTrue(config.isRejectStorageWriteOnLowMemory()); testContext.assertEquals(config.getFreeMemoryCheckIntervalMs(), 30000L); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 250); testContext.assertEquals(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "my-storage");