Skip to content

Commit

Permalink
#173 Add limit for StorageExpand feature
Browse files Browse the repository at this point in the history
  • Loading branch information
mcweba committed May 1, 2024
1 parent 35b482e commit 330952f
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 10 deletions.
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand Down
38 changes: 29 additions & 9 deletions src/main/java/org/swisspush/reststorage/RestStorageHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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);
Expand All @@ -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);
Expand Down Expand Up @@ -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<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;
}

Expand All @@ -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();

Expand Down Expand Up @@ -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());

Check warning on line 672 in src/main/java/org/swisspush/reststorage/RestStorageHandler.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L672 was not covered by tests
}

String mimeType = mimeTypeResolver.resolveMimeType(path);
Expand All @@ -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());

Check warning on line 692 in src/main/java/org/swisspush/reststorage/RestStorageHandler.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L692 was not covered by tests
}
rsp.setStatusCode(StatusCode.NOT_FOUND.getStatusCode());
rsp.setStatusMessage(StatusCode.NOT_FOUND.getStatusMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -445,6 +451,10 @@ public int getMaxRedisWaitingHandlers() {
return maxRedisWaitingHandlers;
}

public int getMaxStorageExpandSubresources() {
return maxStorageExpandSubresources;
}

public JsonObject asJsonObject() {
return JsonObject.mapFrom(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -193,6 +197,7 @@ public void testGetOverriddenAsJsonObject(TestContext testContext){
.confirmCollectionDelete(true)
.rejectStorageWriteOnLowMemory(true)
.resourceCleanupIntervalSec(15)
.maxStorageExpandSubresources(500)
.freeMemoryCheckIntervalMs(5000);

JsonObject json = config.asJsonObject();
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 330952f

Please sign in to comment.