This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit aa698c87cc97325c79e440d13e2155957b45095e
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Wed Jun 3 10:02:43 2020 +0700
JAMES-3182 Reject too deep GetMessageList filters
---
.../integration/GetMessageListMethodTest.java | 31 ++++++++-
.../jmap/draft/methods/GetMessageListMethod.java | 36 +++++-----
.../org/apache/james/jmap/draft/model/Filter.java | 33 +++++++++
.../apache/james/jmap/draft/model/FilterTest.java | 78 ++++++++++++++++++++++
4 files changed, 156 insertions(+), 22 deletions(-)
diff --git
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/GetMessageListMethodTest.java
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/GetMessageListMethodTest.java
index 7ba3326..4dd633f 100644
---
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/GetMessageListMethodTest.java
+++
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/GetMessageListMethodTest.java
@@ -669,7 +669,7 @@ public abstract class GetMessageListMethodTest {
}
@Test
- public void
getMessageListShouldFetchUnreadMessagesInMailboxUsingACombinationOfFilter()
throws Exception {
+ public void getMessageListShouldRejectNestedInMailboxClause() throws
Exception {
MailboxId mailboxId =
mailboxProbe.createMailbox(MailboxPath.forUser(ALICE, "mailbox"));
mailboxProbe.createMailbox(MailboxPath.forUser(ALICE, "othermailbox"));
mailboxProbe.createMailbox(MailboxPath.inbox(ALICE));
@@ -704,6 +704,35 @@ public abstract class GetMessageListMethodTest {
}
@Test
+ public void getMessageListShouldRejectTooDeepFilter() {
+ given()
+ .header("Authorization", aliceAccessToken.asString())
+ .body("[[\"getMessageList\", {\"filter\":" +
+ "{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"operator\":\"AND\"," +
+ " \"conditions\":[{\"isUnread\":\"true\"}" +
+ "]}]}]}]}]}]}]}]}]}]}]}]}]}}, \"#0\"]]")
+ .when()
+ .post("/jmap")
+ .then()
+ .statusCode(200)
+ .body(NAME, equalTo("error"))
+ .body(ARGUMENTS + ".type", equalTo("invalidArguments"))
+ .body(ARGUMENTS + ".description", equalTo("Filter depth is higher
than maximum allowed value 10"));
+ }
+
+ @Test
public void
getMessageListOROperatorShouldReturnMessagesWhichMatchOneOfAllConditions()
throws Exception {
mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE,
ALICE.asString(), "mailbox");
diff --git
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessageListMethod.java
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessageListMethod.java
index 3f4b54f..722a90f 100644
---
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessageListMethod.java
+++
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessageListMethod.java
@@ -33,7 +33,6 @@ import javax.inject.Named;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.james.jmap.draft.model.Filter;
import org.apache.james.jmap.draft.model.FilterCondition;
-import org.apache.james.jmap.draft.model.FilterOperator;
import org.apache.james.jmap.draft.model.GetMessageListRequest;
import org.apache.james.jmap.draft.model.GetMessageListResponse;
import org.apache.james.jmap.draft.model.GetMessagesRequest;
@@ -138,6 +137,14 @@ public class GetMessageListMethod implements Method {
.type("invalidArguments")
.description(e.getMessage())
.build())
+ .build()))
+ .onErrorResume(Filter.TooDeepFilterHierarchyException.class, e
-> Mono.just(JmapResponse.builder()
+ .methodCallId(methodCallId)
+ .responseName(RESPONSE_NAME)
+ .error(ErrorResponse.builder()
+ .type("invalidArguments")
+ .description(e.getMessage())
+ .build())
.build()));
}
@@ -177,34 +184,21 @@ public class GetMessageListMethod implements Method {
}
private boolean containsNestedMailboxFilters(Filter filter) {
- if (filter instanceof FilterOperator) {
- FilterOperator operator = (FilterOperator) filter;
-
- return operator.getConditions()
- .stream()
- .anyMatch(this::containsMailboxFilters);
- }
if (filter instanceof FilterCondition) {
// The condition is not nested
return false;
}
- throw new RuntimeException("Unsupported Filter implementation " +
filter);
+ return containsMailboxFilters(filter);
}
private boolean containsMailboxFilters(Filter filter) {
- if (filter instanceof FilterOperator) {
- FilterOperator operator = (FilterOperator) filter;
-
- return operator.getConditions()
- .stream()
- .anyMatch(this::containsMailboxFilters);
- }
- if (filter instanceof FilterCondition) {
- FilterCondition condition = (FilterCondition) filter;
+ return filter.flatten()
+ .stream()
+ .anyMatch(this::hasMailboxClause);
+ }
- return condition.getInMailboxes().isPresent() ||
condition.getInMailboxes().isPresent();
- }
- throw new RuntimeException("Unsupported Filter implementation " +
filter);
+ private boolean hasMailboxClause(FilterCondition condition) {
+ return condition.getInMailboxes().isPresent() ||
condition.getInMailboxes().isPresent();
}
private Set<MailboxId> buildFilterMailboxesSet(Optional<Filter>
maybeFilter, Function<FilterCondition, Optional<List<String>>>
mailboxListExtractor) {
diff --git
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/Filter.java
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/Filter.java
index 5af1046..4294805 100644
---
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/Filter.java
+++
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/Filter.java
@@ -19,11 +19,44 @@
package org.apache.james.jmap.draft.model;
+import java.util.List;
+import java.util.stream.Stream;
+
import org.apache.james.jmap.draft.json.FilterDeserializer;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.github.steveash.guavate.Guavate;
@JsonDeserialize(using = FilterDeserializer.class)
public interface Filter {
+ class TooDeepFilterHierarchyException extends IllegalArgumentException {
+ TooDeepFilterHierarchyException() {
+ super("Filter depth is higher than maximum allowed value " +
MAX_FILTER_DEPTH);
+ }
+ }
+
+ int MAX_FILTER_DEPTH = 10;
+
String prettyPrint(String indentation);
+
+ default List<FilterCondition> flatten() {
+ return this.flatten(0)
+ .collect(Guavate.toImmutableList());
+ }
+
+ default Stream<FilterCondition> flatten(int depth) {
+ if (depth > MAX_FILTER_DEPTH) {
+ throw new TooDeepFilterHierarchyException();
+ }
+ if (this instanceof FilterOperator) {
+ FilterOperator operator = (FilterOperator) this;
+
+ return operator.getConditions().stream()
+ .flatMap(filter -> filter.flatten(depth + 1));
+ }
+ if (this instanceof FilterCondition) {
+ return Stream.of((FilterCondition) this);
+ }
+ throw new RuntimeException("Unsupported Filter implementation " +
this);
+ }
}
diff --git
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/FilterTest.java
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/FilterTest.java
index 75a73ae..b988822 100644
---
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/FilterTest.java
+++
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/FilterTest.java
@@ -19,6 +19,9 @@
package org.apache.james.jmap.draft.model;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.stream.IntStream;
import org.apache.james.jmap.draft.json.ObjectMapperFactory;
import org.apache.james.mailbox.inmemory.InMemoryId;
@@ -91,4 +94,79 @@ public class FilterTest {
Filter actual = parser.readValue(json, Filter.class);
assertThat(actual).isEqualTo(expected);
}
+
+ @Test
+ public void flattenShouldNoopWhenCondition() {
+ FilterCondition condition = FilterCondition.builder()
+ .to("b...@domain.tld")
+ .build();
+
+ assertThat(condition.flatten(10))
+ .containsExactly(condition);
+ }
+
+ @Test
+ public void flattenShouldUnboxOneLevelOperator() {
+ FilterCondition condition1 = FilterCondition.builder()
+ .to("b...@domain.tld")
+ .build();
+ FilterCondition condition2 = FilterCondition.builder()
+ .to("al...@domain.tld")
+ .build();
+
+ assertThat(FilterOperator.and(condition1, condition2)
+ .flatten())
+ .containsExactly(condition1, condition2);
+ }
+
+ @Test
+ public void flattenShouldUnboxTwoLevelOperator() {
+ FilterCondition condition1 = FilterCondition.builder()
+ .to("b...@domain.tld")
+ .build();
+ FilterCondition condition2 = FilterCondition.builder()
+ .to("al...@domain.tld")
+ .build();
+ FilterCondition condition3 = FilterCondition.builder()
+ .to("ced...@domain.tld")
+ .build();
+
+ assertThat(FilterOperator.and(condition1,
FilterOperator.and(condition2, condition3))
+ .flatten())
+ .containsExactly(condition1, condition2, condition3);
+ }
+
+ @Test
+ public void flattenShouldAllowUpToLimitNesting() {
+ FilterCondition condition = FilterCondition.builder()
+ .to("b...@domain.tld")
+ .build();
+
+ Filter nestedFilter = IntStream.range(0, 10).boxed().reduce(
+ (Filter) condition,
+ (filter, i) -> FilterOperator.and(filter),
+ (f1, f2) -> {
+ throw new RuntimeException("unsuported combinaison");
+ });
+
+ assertThat(nestedFilter.flatten())
+ .containsExactly(condition);
+ }
+
+ @Test
+ public void flattenShouldRejectDeepNesting() {
+ FilterCondition condition = FilterCondition.builder()
+ .to("b...@domain.tld")
+ .build();
+
+ Filter nestedFilter = IntStream.range(0, 11).boxed().reduce(
+ (Filter) condition,
+ (filter, i) -> FilterOperator.and(filter),
+ (f1, f2) -> {
+ throw new RuntimeException("unsuported combinaison");
+ });
+
+ assertThatThrownBy(nestedFilter::flatten)
+ .isInstanceOf(Filter.TooDeepFilterHierarchyException.class);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
