You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/06/04 02:17:59 UTC
[james-project] branch master updated (609ee55 -> 5e5c80f)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.
from 609ee55 JAMES-3184 rename RunningOptionsParser to avoid conflict with other parsers
new db4c62f JAMES-2760 mailqueue.size.metricsEnabled should be false by default
new 5b96391 JAMES-3199: add validate mailboxname and also test at UserMailboxesRoutesTest
new af1f391 JAMES-3182 JMAPResponse Error: Remove ambiguous untyped builder method
new e837471 JAMES−3182 JMAPResponse: fail early upon unspecified responseName/methodCallId
new 35c669e JAMES-3182 Explicitly reject nested mailbox filters for GetMessageList
new aa698c8 JAMES-3182 Reject too deep GetMessageList filters
new 5e5c80f [ADR] Enforce monotic ADR id increment
The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGELOG.md | 1 +
.../destination/conf/rabbitmq.properties | 4 +-
.../destination/conf/rabbitmq.properties | 4 +-
.../HasEmptyMailboxNameInHierarchyException.java | 9 --
.../apache/james/mailbox/model/MailboxPath.java | 6 +-
.../integration/GetMessageListMethodTest.java | 49 ++++++---
.../jmap/draft/methods/GetMessageListMethod.java | 42 +++++++-
.../james/jmap/draft/methods/JmapResponse.java | 10 +-
.../org/apache/james/jmap/draft/model/Filter.java | 33 ++++++
.../apache/james/jmap/draft/model/FilterTest.java | 78 ++++++++++++++
.../james/webadmin/routes/UserMailboxesRoutes.java | 17 +--
.../webadmin/service/UserMailboxesService.java | 15 +--
.../james/webadmin/validation/MailboxName.java | 7 +-
.../webadmin/routes/UserMailboxesRoutesTest.java | 116 ++++++++++++++++-----
.../view/RabbitMQMailQueueConfiguration.java | 4 +-
.../view/RabbitMQMailQueueConfigurationTest.java | 2 +-
...oving-configured-additional-mailboxListeners.md | 2 +-
.../0027-eventBus-error-handling-upon-dispatch.md | 2 +-
.../0035-distributed-listeners-configuration.md | 4 +-
src/adr/{0036-eventbus.md => 0037-eventbus.md} | 4 +-
...ed-eventbus.md => 0038-distributed-eventbus.md} | 4 +-
... => 0039-distributed-blob-garbage-collector.md} | 2 +-
src/site/xdoc/server/config-rabbitmq.xml | 2 +-
upgrade-instructions.md | 14 +++
24 files changed, 335 insertions(+), 96 deletions(-)
rename src/adr/{0036-eventbus.md => 0037-eventbus.md} (98%)
rename src/adr/{0037-distributed-eventbus.md => 0038-distributed-eventbus.md} (94%)
rename src/adr/{0029-distributed-blob-garbage-collector.md => 0039-distributed-blob-garbage-collector.md} (99%)
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 06/07: JAMES-3182 Reject too deep GetMessageList
filters
Posted by bt...@apache.org.
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 <bt...@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("bob@domain.tld")
+ .build();
+
+ assertThat(condition.flatten(10))
+ .containsExactly(condition);
+ }
+
+ @Test
+ public void flattenShouldUnboxOneLevelOperator() {
+ FilterCondition condition1 = FilterCondition.builder()
+ .to("bob@domain.tld")
+ .build();
+ FilterCondition condition2 = FilterCondition.builder()
+ .to("alice@domain.tld")
+ .build();
+
+ assertThat(FilterOperator.and(condition1, condition2)
+ .flatten())
+ .containsExactly(condition1, condition2);
+ }
+
+ @Test
+ public void flattenShouldUnboxTwoLevelOperator() {
+ FilterCondition condition1 = FilterCondition.builder()
+ .to("bob@domain.tld")
+ .build();
+ FilterCondition condition2 = FilterCondition.builder()
+ .to("alice@domain.tld")
+ .build();
+ FilterCondition condition3 = FilterCondition.builder()
+ .to("cedric@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("bob@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("bob@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 01/07: JAMES-2760 mailqueue.size.metricsEnabled
should be false by default
Posted by bt...@apache.org.
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 db4c62fa41737a8a3ca2b4835be6e184c4843796
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Jun 2 15:51:54 2020 +0700
JAMES-2760 mailqueue.size.metricsEnabled should be false by default
This parameter is production friendly and can avoid some incidents.
---
CHANGELOG.md | 1 +
.../destination/conf/rabbitmq.properties | 4 ++--
.../destination/conf/rabbitmq.properties | 4 ++--
.../rabbitmq/view/RabbitMQMailQueueConfiguration.java | 4 ++--
.../rabbitmq/view/RabbitMQMailQueueConfigurationTest.java | 2 +-
src/site/xdoc/server/config-rabbitmq.xml | 2 +-
upgrade-instructions.md | 14 ++++++++++++++
7 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8ce0abd..65505c2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,7 @@ of tasks being currently executed.
- Linshare blob export configuration and mechanism change. See JAMES-3040.
- Differentiation between domain alias and domain mapping. Read upgrade instructions.
- JAMES-3122 Log4J2 adoption for Spring product. Log file configuration needs to be updated. See upgrade instructions.
+- JAMES-2760 mailqueue.size.metricsEnabled should be false by default
### Fixed
- JAMES-2828 & JAMES-2929 bugs affecting JDBCMailRepository usage with PostgresSQL thanks to Jörg Thomas & Sergey B
diff --git a/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/rabbitmq.properties b/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/rabbitmq.properties
index 3d34884..35e9367 100644
--- a/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/rabbitmq.properties
+++ b/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/rabbitmq.properties
@@ -38,5 +38,5 @@ mailqueue.view.updateBrowseStartPace=1000
# Metrics get exported periodically as configured in elasticsearch.properties, thus getSize is also called periodically
# Choose to disable it when the mail queue size is getting too big
# Note that this is as well a temporary workaround until we get 'getSize' method better optimized
-# Optional, default true
-mailqueue.size.metricsEnabled=true
\ No newline at end of file
+# Optional, default false
+mailqueue.size.metricsEnabled=false
\ No newline at end of file
diff --git a/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/rabbitmq.properties b/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/rabbitmq.properties
index 3d34884..35e9367 100644
--- a/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/rabbitmq.properties
+++ b/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/rabbitmq.properties
@@ -38,5 +38,5 @@ mailqueue.view.updateBrowseStartPace=1000
# Metrics get exported periodically as configured in elasticsearch.properties, thus getSize is also called periodically
# Choose to disable it when the mail queue size is getting too big
# Note that this is as well a temporary workaround until we get 'getSize' method better optimized
-# Optional, default true
-mailqueue.size.metricsEnabled=true
\ No newline at end of file
+# Optional, default false
+mailqueue.size.metricsEnabled=false
\ No newline at end of file
diff --git a/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfiguration.java b/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfiguration.java
index b05c0bc..0145517 100644
--- a/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfiguration.java
+++ b/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfiguration.java
@@ -25,7 +25,7 @@ import java.util.Optional;
import org.apache.commons.configuration2.Configuration;
public class RabbitMQMailQueueConfiguration {
- private static final boolean DEFAULT_SIZE_METRICS_ENABLED = true;
+ private static final boolean DEFAULT_SIZE_METRICS_DISABLED = false;
public static class Builder {
private Optional<Boolean> sizeMetricsEnabled;
@@ -41,7 +41,7 @@ public class RabbitMQMailQueueConfiguration {
}
public RabbitMQMailQueueConfiguration build() {
- return new RabbitMQMailQueueConfiguration(sizeMetricsEnabled.orElse(DEFAULT_SIZE_METRICS_ENABLED));
+ return new RabbitMQMailQueueConfiguration(sizeMetricsEnabled.orElse(DEFAULT_SIZE_METRICS_DISABLED));
}
}
diff --git a/server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfigurationTest.java b/server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfigurationTest.java
index fefaff8..e442fac 100644
--- a/server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfigurationTest.java
+++ b/server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/view/RabbitMQMailQueueConfigurationTest.java
@@ -37,7 +37,7 @@ class RabbitMQMailQueueConfigurationTest {
RabbitMQMailQueueConfiguration actual = RabbitMQMailQueueConfiguration.from(new PropertiesConfiguration());
assertThat(actual)
- .isEqualTo(RabbitMQMailQueueConfiguration.sizeMetricsEnabled());
+ .isEqualTo(RabbitMQMailQueueConfiguration.sizeMetricsDisabled());
}
@Test
diff --git a/src/site/xdoc/server/config-rabbitmq.xml b/src/site/xdoc/server/config-rabbitmq.xml
index 9a375da..90ef6ff 100644
--- a/src/site/xdoc/server/config-rabbitmq.xml
+++ b/src/site/xdoc/server/config-rabbitmq.xml
@@ -97,7 +97,7 @@
<dt><strong>mailqueue.size.metricsEnabled</strong></dt>
<dd>
- By default, the metrics are enabled for the mail queue size.
+ By default, the metrics are disabled for the mail queue size.
As computing the size of the mail queue is currently implemented on top of browse operation and thus have a linear complexity,
sometimes it can get too big, making it impossible for the ES reporter to handle it correctly without crashing.
It can be useful then to disable it.
diff --git a/upgrade-instructions.md b/upgrade-instructions.md
index 2065028..a1171f4 100644
--- a/upgrade-instructions.md
+++ b/upgrade-instructions.md
@@ -34,6 +34,18 @@ Change list:
- [Tune Cassandra time serie tables options](#tune-cassandra-time-serie-tables-options)
- [Log4J2 Adoption](#log4j2-adoption)
- [Drop Cassandra schema version prior version 5](#drop-cassandra-schema-version-prior-version-5)
+ - [mailqueue.size.metricsEnabled now defaults to false](#mailqueuesizemetricsenabled-now-defaults-to-false)
+
+### mailqueue.size.metricsEnabled now defaults to false
+
+Date 03/06/2020
+
+JIRA: https://issues.apache.org/jira/browse/JAMES-2760
+
+Concerned product: Distributed James
+
+`mailqueue.size.metricsEnabled` is now false by default. If you previously used it, please be aware that it can have
+some important performance penalty, and set it explicitly to `true` if you still need it.
### Drop Cassandra schema version prior version 5
@@ -41,6 +53,8 @@ Date 06/04/2020
JIRA: https://issues.apache.org/jira/browse/JAMES-2997
+Concerned product: Distributed James, Cassandra-guice James server
+
In an effort to simplify the code base, we dropped support for Cassandra schema version prior version 5.
Installation running older schema version than version 5 needs to rely on release 3.5.0 to upgrade to schema version 7,
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 02/07: JAMES-3199: add validate mailboxname and
also test at UserMailboxesRoutesTest
Posted by bt...@apache.org.
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 5b9639171bbf6da99ec7cc7bf9cbb01d4b0393dd
Author: duc91 <du...@gmail.com>
AuthorDate: Tue Jun 2 10:41:09 2020 +0700
JAMES-3199: add validate mailboxname and also test at UserMailboxesRoutesTest
---
.../HasEmptyMailboxNameInHierarchyException.java | 9 --
.../apache/james/mailbox/model/MailboxPath.java | 6 +-
.../james/webadmin/routes/UserMailboxesRoutes.java | 17 +--
.../webadmin/service/UserMailboxesService.java | 15 +--
.../james/webadmin/validation/MailboxName.java | 7 +-
.../webadmin/routes/UserMailboxesRoutesTest.java | 116 ++++++++++++++++-----
6 files changed, 111 insertions(+), 59 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java b/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java
index 5363c95..fc49e42 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java
@@ -20,16 +20,7 @@
package org.apache.james.mailbox.exception;
public class HasEmptyMailboxNameInHierarchyException extends MailboxNameException {
- public HasEmptyMailboxNameInHierarchyException() {
- super();
- }
-
public HasEmptyMailboxNameInHierarchyException(String message) {
super(message);
}
-
- public HasEmptyMailboxNameInHierarchyException(String message, Throwable cause) {
- super(message, cause);
- }
-
}
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java
index 3c6a8ed..36340ee 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java
@@ -159,9 +159,10 @@ public class MailboxPath {
return this;
}
- public void assertAcceptable(char pathDelimiter) throws MailboxNameException {
+ public MailboxPath assertAcceptable(char pathDelimiter) throws MailboxNameException {
if (hasEmptyNameInHierarchy(pathDelimiter)) {
- throw new HasEmptyMailboxNameInHierarchyException(asString());
+ throw new HasEmptyMailboxNameInHierarchyException(
+ String.format("'%s' has an empty part within its mailbox name considering %s as a delimiter", asString(), pathDelimiter));
}
if (nameContainsForbiddenCharacters()) {
throw new MailboxNameException(asString() + " contains one of the forbidden characters " + INVALID_CHARS);
@@ -169,6 +170,7 @@ public class MailboxPath {
if (isMailboxNameTooLong()) {
throw new TooLongMailboxNameException("Mailbox name exceeds maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
}
+ return this;
}
private boolean nameContainsForbiddenCharacters() {
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
index c714376..a18747d 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
@@ -35,6 +35,7 @@ import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import org.apache.james.core.Username;
+import org.apache.james.mailbox.exception.MailboxNameException;
import org.apache.james.mailbox.indexer.ReIndexer;
import org.apache.james.task.TaskManager;
import org.apache.james.webadmin.Constants;
@@ -217,12 +218,12 @@ public class UserMailboxesRoutes implements Routes {
.message("Attempt to delete a mailbox with children")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to delete an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
- .message("Attempt to create an invalid mailbox")
+ .message("Attempt to delete an invalid mailbox")
.cause(e)
.haltError();
}
@@ -291,12 +292,12 @@ public class UserMailboxesRoutes implements Routes {
.message("Invalid get on user mailboxes")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to test existence of an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
- .message("Attempt to create an invalid mailbox")
+ .message("Attempt to test existence of an invalid mailbox")
.cause(e)
.haltError();
}
@@ -330,8 +331,8 @@ public class UserMailboxesRoutes implements Routes {
.message("Invalid get on user mailboxes")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to create an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
index 003220f..8f4e8e7 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
@@ -64,9 +64,9 @@ public class UserMailboxesService {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
try {
- mailboxManager.createMailbox(
- MailboxPath.forUser(username, mailboxName.asString()),
- mailboxSession);
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
+ mailboxManager.createMailbox(mailboxPath, mailboxSession);
} catch (MailboxExistsException e) {
LOGGER.info("Attempt to create mailbox {} for user {} that already exists", mailboxName, username);
}
@@ -91,16 +91,17 @@ public class UserMailboxesService {
public boolean testMailboxExists(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
- return Mono.from(mailboxManager.mailboxExists(
- MailboxPath.forUser(username, mailboxName.asString()),
- mailboxSession))
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
+ return Mono.from(mailboxManager.mailboxExists(mailboxPath, mailboxSession))
.block();
}
public void deleteMailbox(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException, MailboxHaveChildrenException {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
- MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString());
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
listChildren(mailboxPath, mailboxSession)
.forEach(Throwing.consumer(path -> deleteMailbox(mailboxSession, path)));
}
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
index fdd079d..35d3844 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
@@ -19,18 +19,15 @@
package org.apache.james.webadmin.validation;
-import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
public class MailboxName {
-
- public static final CharMatcher INVALID_CHARS_MATCHER = CharMatcher.anyOf("%*&#");
private final String mailboxName;
public MailboxName(String mailboxName) {
- Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
- Preconditions.checkArgument(INVALID_CHARS_MATCHER.matchesNoneOf(mailboxName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName), "MailboxName must not be null or empty");
+
this.mailboxName = mailboxName;
}
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index 5d21359..35f8374 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -90,6 +90,8 @@ import reactor.core.publisher.Mono;
class UserMailboxesRoutesTest {
private static final Username USERNAME = Username.of("username");
private static final String MAILBOX_NAME = "myMailboxName";
+ private static final String MAILBOX_NAME_WITH_DOTS = "my..MailboxName";
+ private static final String INVALID_MAILBOX_NAME = "myMailboxName#";
private static final MailboxPath INBOX = MailboxPath.inbox(USERNAME);
private WebAdminServer webAdminServer;
@@ -203,6 +205,84 @@ class UserMailboxesRoutesTest {
}
@Test
+ void putShouldThrowWhenMailboxNameWithDots() throws Exception {
+ Map<String, Object> errors = when()
+ .put(MAILBOX_NAME_WITH_DOTS)
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:my..MailboxName' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenMailboxNameStartsWithDot() throws Exception {
+ Map<String, Object> errors = when()
+ .put(".startWithDot")
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:.startWithDot' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenMailboxNameEndsWithDots() throws Exception {
+ Map<String, Object> errors = when()
+ .put("endWithDot.")
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:endWithDot.' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenInvalidMailboxName() throws Exception {
+ when(usersRepository.contains(USERNAME)).thenReturn(true);
+
+ Map<String, Object> errors = when()
+ .put(INVALID_MAILBOX_NAME)
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName# contains one of the forbidden characters %*&#");
+ }
+
+ @Test
void deleteShouldReturnNotFoundWithNonExistingUser() throws Exception {
when(usersRepository.contains(USERNAME)).thenReturn(false);
@@ -224,8 +304,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "*")
.then()
@@ -239,13 +317,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName* contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "*")
.then()
@@ -264,8 +341,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "*")
.then()
@@ -284,8 +359,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "%")
.then()
@@ -299,13 +372,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName% contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "%")
.then()
@@ -324,8 +396,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "%")
.then()
@@ -344,8 +414,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "#")
.then()
@@ -359,13 +427,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName# contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "#")
.then()
@@ -384,8 +451,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "#")
.then()
@@ -404,8 +469,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "&")
.then()
@@ -419,13 +482,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName& contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "&")
.then()
@@ -444,8 +506,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "&")
.then()
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 05/07: JAMES-3182 Explicitly reject nested mailbox
filters for GetMessageList
Posted by bt...@apache.org.
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 35c669e3e5120e3c3334e7a6ebfb72b4146f75aa
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Jun 1 13:17:03 2020 +0700
JAMES-3182 Explicitly reject nested mailbox filters for GetMessageList
---
.../integration/GetMessageListMethodTest.java | 18 +++-----
.../jmap/draft/methods/GetMessageListMethod.java | 48 +++++++++++++++++++++-
2 files changed, 53 insertions(+), 13 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 6464d1d..7ba3326 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
@@ -94,7 +94,6 @@ import org.awaitility.Duration;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -669,12 +668,11 @@ public abstract class GetMessageListMethodTest {
messageNotSeenFlaggedInOtherMailbox.getMessageId().serialize()))));
}
- @Ignore("To fix in JAMES-3182")
- @Category(BasicFeature.class)
@Test
public void getMessageListShouldFetchUnreadMessagesInMailboxUsingACombinationOfFilter() throws Exception {
- MailboxId mailboxId = mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, ALICE.asString(), "mailbox");
- mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, ALICE.asString(), "othermailbox");
+ MailboxId mailboxId = mailboxProbe.createMailbox(MailboxPath.forUser(ALICE, "mailbox"));
+ mailboxProbe.createMailbox(MailboxPath.forUser(ALICE, "othermailbox"));
+ mailboxProbe.createMailbox(MailboxPath.inbox(ALICE));
ComposedMessageId messageNotSeenNotFlagged = mailboxProbe.appendMessage(ALICE.asString(), ALICE_MAILBOX,
new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes()), new Date(), false, new Flags());
@@ -700,13 +698,9 @@ public abstract class GetMessageListMethodTest {
.post("/jmap")
.then()
.statusCode(200)
- .body(NAME, equalTo("messageList"))
- .body(ARGUMENTS + ".messageIds", allOf(
- containsInAnyOrder(messageNotSeenNotFlagged.getMessageId().serialize(), messageNotSeenFlagged.getMessageId().serialize()),
- not(containsInAnyOrder(messageSeenNotFlagged.getMessageId().serialize(),
- messageSeenFlagged.getMessageId().serialize(),
- messageSeenInOtherMailbox.getMessageId().serialize(),
- messageNotSeenFlaggedInOtherMailbox.getMessageId().serialize()))));
+ .body(NAME, equalTo("error"))
+ .body(ARGUMENTS + ".type", equalTo("invalidArguments"))
+ .body(ARGUMENTS + ".description", equalTo("'inMailboxes' and 'notInMailboxes' wrapped within Filter Operators are not implemented. Review your search request."));
}
@Test
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 cd31826..3f4b54f 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
@@ -30,8 +30,10 @@ import java.util.stream.Stream;
import javax.inject.Inject;
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;
@@ -128,7 +130,15 @@ public class GetMessageListMethod implements Method {
.response(messageListResponse)
.responseName(RESPONSE_NAME)
.build()),
- processGetMessages(messageListRequest, messageListResponse, methodCallId, mailboxSession)));
+ processGetMessages(messageListRequest, messageListResponse, methodCallId, mailboxSession)))
+ .onErrorResume(NotImplementedException.class, e -> Mono.just(JmapResponse.builder()
+ .methodCallId(methodCallId)
+ .responseName(RESPONSE_NAME)
+ .error(ErrorResponse.builder()
+ .type("invalidArguments")
+ .description(e.getMessage())
+ .build())
+ .build()));
}
private Mono<GetMessageListResponse> getMessageListResponse(GetMessageListRequest messageListRequest, MailboxSession mailboxSession) {
@@ -145,6 +155,11 @@ public class GetMessageListMethod implements Method {
}
private MultimailboxesSearchQuery convertToSearchQuery(GetMessageListRequest messageListRequest) {
+ if (messageListRequest.getFilter().map(this::containsNestedMailboxFilters).orElse(false)) {
+ throw new NotImplementedException("'inMailboxes' and 'notInMailboxes' wrapped within Filter Operators are not " +
+ "implemented. Review your search request.");
+ }
+
SearchQuery searchQuery = messageListRequest.getFilter()
.map(filter -> new FilterToSearchQuery().convert(filter))
.orElse(new SearchQuery());
@@ -161,6 +176,37 @@ public class GetMessageListMethod implements Method {
.build();
}
+ 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);
+ }
+
+ 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 condition.getInMailboxes().isPresent() || condition.getInMailboxes().isPresent();
+ }
+ throw new RuntimeException("Unsupported Filter implementation " + filter);
+ }
+
private Set<MailboxId> buildFilterMailboxesSet(Optional<Filter> maybeFilter, Function<FilterCondition, Optional<List<String>>> mailboxListExtractor) {
return filterToFilterCondition(maybeFilter)
.flatMap(condition -> Guavate.stream(mailboxListExtractor.apply(condition)))
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 04/07: JAMES−3182 JMAPResponse: fail early upon unspecified responseName/methodCallId
Posted by bt...@apache.org.
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 e837471c745483b707b3bf1b7fb54063edb1d47f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Jun 1 12:03:54 2020 +0700
JAMES−3182 JMAPResponse: fail early upon unspecified responseName/methodCallId
Currently we build an invalid object we cannot write to the client:
```
12:00:11.515 [ERROR] o.a.j.j.h.JMAPApiRoutes - Unexpected error
java.lang.NullPointerException: methodCallId is mandatory
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:895)
at org.apache.james.jmap.draft.model.InvocationResponse.<init>(InvocationResponse.java:35)
at org.apache.james.jmap.draft.methods.JmapResponseWriterImpl.lambda$formatMethodResponse$0(JmapResponseWriterImpl.java:58)
```
---
.../main/java/org/apache/james/jmap/draft/methods/JmapResponse.java | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
index 8113098..2706e25 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
@@ -28,6 +28,7 @@ import org.apache.james.jmap.draft.model.Property;
import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
public class JmapResponse {
@@ -90,6 +91,9 @@ public class JmapResponse {
public JmapResponse build() {
+ Preconditions.checkState(methodCallId != null, "'methodCallId' needs to be specified");
+ Preconditions.checkState(responseName != null, "'responseName' needs to be specified");
+
return new JmapResponse(responseName, methodCallId, response, properties, filterProvider);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 07/07: [ADR] Enforce monotic ADR id increment
Posted by bt...@apache.org.
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 5e5c80f9171438f5af387c5be7511a68f678716f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri May 29 10:24:56 2020 +0700
[ADR] Enforce monotic ADR id increment
---
src/adr/0026-removing-configured-additional-mailboxListeners.md | 2 +-
src/adr/0027-eventBus-error-handling-upon-dispatch.md | 2 +-
src/adr/0035-distributed-listeners-configuration.md | 4 ++--
src/adr/{0036-eventbus.md => 0037-eventbus.md} | 4 ++--
.../{0037-distributed-eventbus.md => 0038-distributed-eventbus.md} | 4 ++--
...arbage-collector.md => 0039-distributed-blob-garbage-collector.md} | 2 +-
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/adr/0026-removing-configured-additional-mailboxListeners.md b/src/adr/0026-removing-configured-additional-mailboxListeners.md
index f81c59b..5eefb81 100644
--- a/src/adr/0026-removing-configured-additional-mailboxListeners.md
+++ b/src/adr/0026-removing-configured-additional-mailboxListeners.md
@@ -19,7 +19,7 @@ The distributed James server then declares a queue upon start for each one of th
binds to the main event exchange.
More information about this component, and its distributed, RabbitMQ based implementation, can be found in
-[ADR 0036](0036-eventbus.md).
+[ADR 0036](0037-eventbus.md).
If the user unconfigures the listener, the queue and the binding are still present but not consumed. This results in
unbounded queue growth eventually causing RabbitMQ resource exhaustion and failure.
diff --git a/src/adr/0027-eventBus-error-handling-upon-dispatch.md b/src/adr/0027-eventBus-error-handling-upon-dispatch.md
index 053b713..bdb36ff 100644
--- a/src/adr/0027-eventBus-error-handling-upon-dispatch.md
+++ b/src/adr/0027-eventBus-error-handling-upon-dispatch.md
@@ -19,7 +19,7 @@ order to retry their processing via webAdmin APIs.
However no such capabilities are supported upon dispatching the event on the eventbus. A failed dispatch will result in message loss.
-More information about this component can be found in [ADR 0036](0036-eventbus.md).
+More information about this component can be found in [ADR 0036](0037-eventbus.md).
## Decision
diff --git a/src/adr/0035-distributed-listeners-configuration.md b/src/adr/0035-distributed-listeners-configuration.md
index 78ada23..120e7ad 100644
--- a/src/adr/0035-distributed-listeners-configuration.md
+++ b/src/adr/0035-distributed-listeners-configuration.md
@@ -19,7 +19,7 @@ Mailbox listeners can be registered to be triggered every time an event is gener
mailbox. They are being executed in a distributed fashion following the workqueue messaging pattern. The "group" is an
attribute of the mailbox listener identifying to which work queue they belong.
-More information about this component can be found in [ADR 0036](0036-eventbus.md).
+More information about this component can be found in [ADR 0036](0037-eventbus.md).
Currently, mailbox listeners are determined by the guice bindings of the server and additional mailbox listeners defined
via configuration files.
@@ -78,7 +78,7 @@ A WebAdmin endpoint will allow:
A broadcast on the event bus will be attempted to propagate topology changes, by the mean of a common registrationKey
to all nodes, a "TopologyChanged" event, and a mailbox listener starting the MailboxListeners on local node upon
-topology changes. `registrationKey` concept is explained in [ADR 0036](0036-eventbus.md).
+topology changes. `registrationKey` concept is explained in [ADR 0036](0037-eventbus.md).
If a listener is added but is not in the classpath, an ERROR log is emitted. This can happen during a rolling upgrade,
which defines a new guice binding for a new mailbox listener. Events will still be emitted (and consumed by other James
diff --git a/src/adr/0036-eventbus.md b/src/adr/0037-eventbus.md
similarity index 98%
rename from src/adr/0036-eventbus.md
rename to src/adr/0037-eventbus.md
index e16f782..f52b9f8 100644
--- a/src/adr/0036-eventbus.md
+++ b/src/adr/0037-eventbus.md
@@ -1,4 +1,4 @@
-# 36. Event bus
+# 37. Event bus
Date: 2020-05-05
@@ -48,7 +48,7 @@ key it is registered to, in an at least once fashion.
We need to provide an `In VM` implementation of the EventBus for single server deployments.
-We also need to provide [a distributed event bus implementation](0037-distributed-eventbus.md).
+We also need to provide [a distributed event bus implementation](0038-distributed-eventbus.md).
## Current usages
diff --git a/src/adr/0037-distributed-eventbus.md b/src/adr/0038-distributed-eventbus.md
similarity index 94%
rename from src/adr/0037-distributed-eventbus.md
rename to src/adr/0038-distributed-eventbus.md
index eea132e..cb02ac6 100644
--- a/src/adr/0037-distributed-eventbus.md
+++ b/src/adr/0038-distributed-eventbus.md
@@ -1,4 +1,4 @@
-# 37. Distributed Event bus
+# 38. Distributed Event bus
Date: 2020-05-25
@@ -8,7 +8,7 @@ Accepted (lazy consensus)
## Context
-Read [Event Bus ADR](0036-eventbus.md) for context.
+Read [Event Bus ADR](0037-eventbus.md) for context.
Given several James servers, we need them to share a common EventBus.
diff --git a/src/adr/0029-distributed-blob-garbage-collector.md b/src/adr/0039-distributed-blob-garbage-collector.md
similarity index 99%
rename from src/adr/0029-distributed-blob-garbage-collector.md
rename to src/adr/0039-distributed-blob-garbage-collector.md
index 36b3bdc..d2bcca1 100644
--- a/src/adr/0029-distributed-blob-garbage-collector.md
+++ b/src/adr/0039-distributed-blob-garbage-collector.md
@@ -1,4 +1,4 @@
-# 29. Distributed blob garbage collector
+# 39. Distributed blob garbage collector
Date: 2020-02-18
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 03/07: JAMES-3182 JMAPResponse Error: Remove
ambiguous untyped builder method
Posted by bt...@apache.org.
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 af1f391a08175090b1e22bab679b87d7b4abcaaa
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Jun 1 11:56:40 2020 +0700
JAMES-3182 JMAPResponse Error: Remove ambiguous untyped builder method
I assumed from method signature that the string parameter was a message
while it was used for type.
I removed this unused ambiguous method about to make me introduce a bug
---
.../main/java/org/apache/james/jmap/draft/methods/JmapResponse.java | 6 ------
1 file changed, 6 deletions(-)
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
index 3174e95..8113098 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
@@ -81,12 +81,6 @@ public class JmapResponse {
this.responseName = ErrorResponse.ERROR_METHOD;
return this;
}
-
- public Builder error(String message) {
- this.response = ErrorResponse.builder().type(message).build();
- this.responseName = ErrorResponse.ERROR_METHOD;
- return this;
- }
public Builder error(ErrorResponse error) {
this.response = error;
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org