You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2023/11/02 07:51:37 UTC
(james-project) branch master updated: JAMES-3944 FILTER: forward loop issue (#1762)
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
The following commit(s) were added to refs/heads/master by this push:
new 55454b26a3 JAMES-3944 FILTER: forward loop issue (#1762)
55454b26a3 is described below
commit 55454b26a37c3575c3f6a1b23adebfba0d1c2ad0
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Thu Nov 2 08:51:32 2023 +0100
JAMES-3944 FILTER: forward loop issue (#1762)
Prevents infinite email loop with forwards by tracking for which users the forwards where applied.
Tracks both regular forwards and JMAP filters forwards.
Co-authored-by: hung phan <hp...@linagora.com>
---
mailet/api/pom.xml | 5 +
.../java/org/apache/mailet/LoopPrevention.java | 109 ++++++++++
.../java/org/apache/mailet/LoopPreventionTest.java | 90 ++++++++
.../james/utils/FilteringManagementProbeImpl.java | 10 +
.../james/utils/MailRepositoryProbeImpl.java | 6 +
.../james/jmap/cassandra/filtering/RuleDTO.java | 8 +-
.../org/apache/james/jmap/api/filtering/Rule.java | 40 +++-
.../james/jmap/api/filtering/RuleFixture.java | 6 +-
.../mailets/FilterForwardIntegrationTest.java | 87 ++------
.../james/mailets/ForwardIntegrationTest.java | 12 +-
.../james/mailets/ForwardLoopIntegrationTest.java | 238 +++++++++++++++++++++
.../mailets/RecipientRewriteTableProcessor.java | 83 ++++---
.../james/jmap/mailet/filter/ActionApplier.java | 145 +++++++++----
13 files changed, 676 insertions(+), 163 deletions(-)
diff --git a/mailet/api/pom.xml b/mailet/api/pom.xml
index afb5d74e6b..f81158c06b 100644
--- a/mailet/api/pom.xml
+++ b/mailet/api/pom.xml
@@ -71,6 +71,11 @@
<groupId>com.sun.mail</groupId>
<artifactId>javax.mail</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
diff --git a/mailet/api/src/main/java/org/apache/mailet/LoopPrevention.java b/mailet/api/src/main/java/org/apache/mailet/LoopPrevention.java
new file mode 100644
index 0000000000..4172a14234
--- /dev/null
+++ b/mailet/api/src/main/java/org/apache/mailet/LoopPrevention.java
@@ -0,0 +1,109 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.mailet;
+
+import java.util.Collection;
+import java.util.Set;
+
+import org.apache.james.core.MailAddress;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+
+public class LoopPrevention {
+ public static class RecordedRecipients {
+ public static final RecordedRecipients NO_RECORDED_RECIPIENTS = new RecordedRecipients(ImmutableSet.of());
+
+ public static RecordedRecipients fromMail(Mail mail) {
+ return mail.getAttribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME)
+ .map(RecordedRecipients::fromAttribute)
+ .orElse(NO_RECORDED_RECIPIENTS);
+ }
+
+ public static RecordedRecipients fromAttribute(Attribute attribute) {
+ Collection<AttributeValue> attributeValues = (Collection<AttributeValue>) attribute.getValue().getValue();
+ return new RecordedRecipients(attributeValues
+ .stream()
+ .map(Throwing.function(attributeValue -> new MailAddress((String) attributeValue.getValue())))
+ .collect(ImmutableSet.toImmutableSet()));
+ }
+
+ private final Set<MailAddress> recipients;
+
+ public RecordedRecipients(Set<MailAddress> recipients) {
+ this.recipients = recipients;
+ }
+
+ public RecordedRecipients(MailAddress... recipients) {
+ this.recipients = ImmutableSet.copyOf(recipients);
+ }
+
+ public Set<MailAddress> getRecipients() {
+ return recipients;
+ }
+
+ public Set<MailAddress> nonRecordedRecipients(Collection<MailAddress> recipients) {
+ return Sets.difference(ImmutableSet.copyOf(recipients), this.recipients);
+ }
+
+ public Set<MailAddress> nonRecordedRecipients(MailAddress... recipients) {
+ return Sets.difference(ImmutableSet.copyOf(recipients), this.recipients);
+ }
+
+ public RecordedRecipients merge(RecordedRecipients other) {
+ return new RecordedRecipients(ImmutableSet.<MailAddress>builder()
+ .addAll(recipients)
+ .addAll(other.recipients)
+ .build());
+ }
+
+ public RecordedRecipients merge(Collection<MailAddress> other) {
+ return merge(new RecordedRecipients(ImmutableSet.copyOf(other)));
+ }
+
+ public RecordedRecipients merge(MailAddress... other) {
+ return merge(ImmutableSet.copyOf(other));
+ }
+
+ public RecordedRecipients mergeIfEmpty(MailAddress... other) {
+ if (recipients.isEmpty()) {
+ return merge(ImmutableSet.copyOf(other));
+ } else {
+ return this;
+ }
+ }
+
+ public Attribute asAttribute() {
+ return new Attribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME,
+ AttributeValue.of(recipients.stream()
+ .map(mailAddress -> AttributeValue.of(mailAddress.asString()))
+ .collect(ImmutableList.toImmutableList())));
+ }
+
+ public void recordOn(Mail mail) {
+ mail.setAttribute(asAttribute());
+ }
+ }
+
+ public static final AttributeName RECORDED_RECIPIENTS_ATTRIBUTE_NAME = AttributeName.of("loop.prevention.recorded.recipients");
+
+}
diff --git a/mailet/api/src/test/java/org/apache/mailet/LoopPreventionTest.java b/mailet/api/src/test/java/org/apache/mailet/LoopPreventionTest.java
new file mode 100644
index 0000000000..b99aaaa8b2
--- /dev/null
+++ b/mailet/api/src/test/java/org/apache/mailet/LoopPreventionTest.java
@@ -0,0 +1,90 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.mailet;
+
+import static org.apache.mailet.LoopPrevention.RECORDED_RECIPIENTS_ATTRIBUTE_NAME;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.junit.jupiter.api.Test;
+
+import com.google.common.collect.ImmutableList;
+
+public class LoopPreventionTest {
+
+ public static final String DEFAULT_DOMAIN = "james.org";
+ private static final Username ALICE = Username.of("alice@" + DEFAULT_DOMAIN);
+ private static final Username BOB = Username.of("bob@" + DEFAULT_DOMAIN);
+ private static final Username CEDRIC = Username.of("cedric@" + DEFAULT_DOMAIN);
+
+ @Test
+ void nonRecordedRecipientsShouldWork() throws Exception {
+ assertThat(new LoopPrevention.RecordedRecipients(BOB.asMailAddress())
+ .nonRecordedRecipients(ALICE.asMailAddress(), BOB.asMailAddress()))
+ .containsOnly(ALICE.asMailAddress());
+ }
+
+ @Test
+ void recordedRecipientsShouldWork() throws Exception {
+ Mail mail = mock(Mail.class);
+
+ new LoopPrevention.RecordedRecipients(ALICE.asMailAddress(), BOB.asMailAddress())
+ .merge(CEDRIC.asMailAddress())
+ .recordOn(mail);
+
+ Attribute attribute = new Attribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME,
+ AttributeValue.of(ImmutableList.of(AttributeValue.of(ALICE.asString()),
+ AttributeValue.of(BOB.asString()),
+ AttributeValue.of(CEDRIC.asString()))));
+
+ verify(mail).setAttribute(attribute);
+ }
+
+ @Test
+ void fromMailShouldRetrieveRecordedRecipients() throws Exception {
+ Mail mail = mock(Mail.class);
+
+ Attribute attribute = new Attribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME,
+ AttributeValue.of(ImmutableList.of(AttributeValue.of(ALICE.asString()))));
+
+ when(mail.getAttribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME))
+ .thenReturn(Optional.of(attribute));
+
+ assertThat(LoopPrevention.RecordedRecipients.fromMail(mail).getRecipients())
+ .containsOnly(ALICE.asMailAddress());
+ }
+
+ @Test
+ void retrieveRecordedRecipientsShouldReturnEmptyWhenTheAttributeDoesNotExist() {
+ Mail mail = mock(Mail.class);
+
+ when(mail.getAttribute(RECORDED_RECIPIENTS_ATTRIBUTE_NAME))
+ .thenReturn(Optional.empty());
+
+ assertThat(LoopPrevention.RecordedRecipients.fromMail(mail).getRecipients())
+ .isEmpty();
+ }
+
+}
diff --git a/server/container/guice/common/src/main/java/org/apache/james/utils/FilteringManagementProbeImpl.java b/server/container/guice/common/src/main/java/org/apache/james/utils/FilteringManagementProbeImpl.java
index a95039bf57..236dfb08fb 100644
--- a/server/container/guice/common/src/main/java/org/apache/james/utils/FilteringManagementProbeImpl.java
+++ b/server/container/guice/common/src/main/java/org/apache/james/utils/FilteringManagementProbeImpl.java
@@ -20,6 +20,7 @@
package org.apache.james.utils;
import java.util.Optional;
+import java.util.stream.Stream;
import javax.inject.Inject;
@@ -28,6 +29,8 @@ import org.apache.james.jmap.api.filtering.FilteringManagement;
import org.apache.james.jmap.api.filtering.Rule;
import org.apache.james.jmap.api.filtering.Version;
+import com.google.common.collect.ImmutableList;
+
import reactor.core.publisher.Mono;
public class FilteringManagementProbeImpl implements GuiceProbe {
@@ -42,4 +45,11 @@ public class FilteringManagementProbeImpl implements GuiceProbe {
public void defineRulesForUser(Username username, Optional<Version> ifInState, Rule... rules) {
Mono.from(filteringManagement.defineRulesForUser(username, ifInState, rules)).block();
}
+
+ public void defineRulesForUser(Username username, Rule.Builder... rules) {
+ Optional<Version> noVersion = Optional.empty();
+ Mono.from(filteringManagement.defineRulesForUser(username, Stream.of(rules)
+ .map(Rule.Builder::build)
+ .collect(ImmutableList.toImmutableList()), noVersion)).block();
+ }
}
diff --git a/server/container/guice/common/src/main/java/org/apache/james/utils/MailRepositoryProbeImpl.java b/server/container/guice/common/src/main/java/org/apache/james/utils/MailRepositoryProbeImpl.java
index 55f84600a0..925bfb3957 100644
--- a/server/container/guice/common/src/main/java/org/apache/james/utils/MailRepositoryProbeImpl.java
+++ b/server/container/guice/common/src/main/java/org/apache/james/utils/MailRepositoryProbeImpl.java
@@ -24,6 +24,7 @@ import java.util.stream.Stream;
import javax.inject.Inject;
+import org.apache.james.core.MailAddress;
import org.apache.james.mailrepository.api.MailKey;
import org.apache.james.mailrepository.api.MailRepositoryStore;
import org.apache.james.mailrepository.api.MailRepositoryUrl;
@@ -64,6 +65,11 @@ public class MailRepositoryProbeImpl implements GuiceProbe {
.map(Throwing.function(key -> getMail(url, key)));
}
+ public List<Mail> listMails(MailRepositoryUrl url, MailAddress recipient) throws Exception {
+ return listMails(url).filter(Throwing.predicate(mail -> mail.getRecipients().contains(recipient)))
+ .collect(ImmutableList.toImmutableList());
+ }
+
public Mail getMail(MailRepositoryUrl url, MailKey key) throws Exception {
return repositoryStore.select(url)
.retrieve(key);
diff --git a/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/filtering/RuleDTO.java b/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/filtering/RuleDTO.java
index 17c47d3d8e..543bf1dc7a 100644
--- a/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/filtering/RuleDTO.java
+++ b/server/data/data-jmap-cassandra/src/main/java/org/apache/james/jmap/cassandra/filtering/RuleDTO.java
@@ -200,10 +200,10 @@ public class RuleDTO {
}
public Rule.Action.Forward toForward() {
- return Rule.Action.Forward.of(addresses.stream()
- .map(Throwing.function(MailAddress::new))
- .collect(ImmutableList.toImmutableList()),
- keepACopy);
+ return Rule.Action.Forward.to(addresses.stream()
+ .map(Throwing.function(MailAddress::new))
+ .collect(ImmutableList.toImmutableList()))
+ .keepACopy(keepACopy);
}
@Override
diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/Rule.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/Rule.java
index 170193d9ca..645c1705ad 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/Rule.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/Rule.java
@@ -286,6 +286,27 @@ public class Rule {
}
public static class Forward {
+ @FunctionalInterface
+ public interface RequireLocalCopy {
+ Forward keepACopy(boolean value);
+
+ default Forward keepACopy() {
+ return keepACopy(true);
+ }
+
+ default Forward withoutACopy() {
+ return keepACopy(false);
+ }
+ }
+
+ public static RequireLocalCopy to(Collection<MailAddress> addresses) {
+ return keepACopy -> new Forward(ImmutableList.copyOf(addresses), keepACopy);
+ }
+
+ public static RequireLocalCopy to(MailAddress... addresses) {
+ return keepACopy -> new Forward(ImmutableList.copyOf(addresses), keepACopy);
+ }
+
public static Forward of(List<MailAddress> addresses, boolean keepACopy) {
return new Forward(ImmutableList.copyOf(addresses), keepACopy);
}
@@ -368,10 +389,18 @@ public class Rule {
return this;
}
+ public Builder setForward(Forward forward) {
+ this.forward = Optional.of(forward);
+ return this;
+ }
+
public Action build() {
- Preconditions.checkNotNull(appendInMailboxes, "appendInMailboxes should not be null");
- Preconditions.checkNotNull(withKeywords, "withKeywords should not be null");
- Preconditions.checkNotNull(forward, "forward should not be null");
+ appendInMailboxes = Optional.ofNullable(appendInMailboxes)
+ .orElse(Rule.Action.AppendInMailboxes.withMailboxIds(ImmutableList.of()));
+ withKeywords = Optional.ofNullable(withKeywords)
+ .orElse(ImmutableList.of());
+ forward = Optional.ofNullable(forward)
+ .orElse(Optional.empty());
return new Action(appendInMailboxes, markAsSeen, markAsImportant, reject, withKeywords, forward);
}
@@ -499,6 +528,11 @@ public class Rule {
return this;
}
+ public Builder action(Action.Builder action) {
+ this.action = action.build();
+ return this;
+ }
+
public Rule build() {
Preconditions.checkState(id != null, "`id` is mandatory");
Preconditions.checkState(StringUtils.isNotBlank(name), "`name` is mandatory");
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
index 6113d5d5ca..1b3481183a 100644
--- a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
@@ -41,11 +41,11 @@ public interface RuleFixture {
true,
true,
ImmutableList.of("abc"),
- Optional.of(Rule.Action.Forward.of(ImmutableList.of("abc@example.com")
+ Optional.of(Rule.Action.Forward.to(ImmutableList.of("abc@example.com")
.stream()
.map(Throwing.function(MailAddress::new))
- .collect(ImmutableList.toImmutableList()),
- true)));
+ .collect(ImmutableList.toImmutableList()))
+ .keepACopy()));
Rule.Builder RULE_BUILDER = Rule.builder().name(NAME).conditionGroup(CONDITION).action(ACTION);
Rule RULE_1 = RULE_BUILDER.id(Rule.Id.of("1")).build();
Rule RULE_1_MODIFIED = Rule.builder()
diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/FilterForwardIntegrationTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/FilterForwardIntegrationTest.java
index 4e4bc8efc7..a3e8654734 100644
--- a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/FilterForwardIntegrationTest.java
+++ b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/FilterForwardIntegrationTest.java
@@ -26,11 +26,11 @@ import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
import static org.apache.james.mailets.configuration.Constants.PASSWORD;
import java.io.File;
-import java.util.Optional;
-import org.apache.james.core.MailAddress;
import org.apache.james.core.Username;
import org.apache.james.jmap.api.filtering.Rule;
+import org.apache.james.jmap.api.filtering.Rule.Action;
+import org.apache.james.jmap.api.filtering.Rule.Action.Forward;
import org.apache.james.jmap.mailet.filter.JMAPFiltering;
import org.apache.james.mailets.configuration.CommonProcessors;
import org.apache.james.mailets.configuration.MailetConfiguration;
@@ -56,15 +56,24 @@ import org.junit.jupiter.api.io.TempDir;
import org.testcontainers.shaded.org.awaitility.Awaitility;
import com.github.fge.lambdas.Throwing;
-import com.google.common.collect.ImmutableList;
import com.google.inject.multibindings.Multibinder;
public class FilterForwardIntegrationTest {
private static final Username ALICE = Username.of("alice@" + DEFAULT_DOMAIN);
+ public static final Rule.ConditionGroup CONDITION_GROUP = Rule.ConditionGroup.of(Rule.ConditionCombiner.AND, Rule.Condition.of(FROM, CONTAINS, ALICE.asString()));
private static final Username BOB = Username.of("bob@" + DEFAULT_DOMAIN);
private static final Username CEDRIC = Username.of("cedric@" + DEFAULT_DOMAIN);
private static final MailRepositoryUrl CUSTOM_REPOSITORY = MailRepositoryUrl.from("memory://var/mail/custom/");
+
+ private static Rule.Builder asRule(Action.Forward forward) {
+ return Rule.builder()
+ .id(Rule.Id.of("1"))
+ .name("rule 1")
+ .conditionGroup(CONDITION_GROUP)
+ .action(Action.builder().setForward(forward));
+ }
+
private TemporaryJamesServer jamesServer;
private FilteringManagementProbeImpl filteringManagementProbe;
private MailRepositoryProbeImpl mailRepositoryProbe;
@@ -111,82 +120,26 @@ public class FilterForwardIntegrationTest {
@Test
void forwardShouldWork() throws Exception {
- ImmutableList<MailAddress> forwardedMailAddresses = ImmutableList.of(CEDRIC.asMailAddress());
- Rule.Action.Forward forward = Rule.Action.Forward.of(forwardedMailAddresses, true);
- filteringManagementProbe.defineRulesForUser(BOB,
- Optional.empty(),
- Rule.builder()
- .id(Rule.Id.of("1"))
- .name("rule 1")
- .conditionGroup(Rule.ConditionGroup.of(Rule.ConditionCombiner.AND, Rule.Condition.of(FROM, CONTAINS, ALICE.asString())))
- .action(Rule.Action.builder().setAppendInMailboxes(Rule.Action.AppendInMailboxes.withMailboxIds(ImmutableList.of()))
- .setWithKeywords(ImmutableList.of())
- .setForward(Optional.of(forward))
- .build())
- .build());
+ filteringManagementProbe.defineRulesForUser(BOB, asRule(Forward.to(CEDRIC.asMailAddress()).keepACopy()));
messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
.authenticate(ALICE.asString(), PASSWORD)
.sendMessage(ALICE.asString(), BOB.asString());
- Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 2L);
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
Mail mail1 = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
- .filter(Throwing.predicate(mail -> mail.getRecipients().contains(CEDRIC.asMailAddress())))
.findAny().get();
- softly.assertThat(mail1.getRecipients()).containsOnly(CEDRIC.asMailAddress());
+ softly.assertThat(mail1.getRecipients()).containsOnly(BOB.asMailAddress(), CEDRIC.asMailAddress());
softly.assertThat(mail1.getMaybeSender().asOptional()).contains(BOB.asMailAddress());
}));
}
- @Test
- void forwardShouldKeepACopy() throws Exception {
- ImmutableList<MailAddress> forwardedMailAddresses = ImmutableList.of(CEDRIC.asMailAddress());
- Rule.Action.Forward forward = Rule.Action.Forward.of(forwardedMailAddresses, true);
- filteringManagementProbe.defineRulesForUser(BOB,
- Optional.empty(),
- Rule.builder()
- .id(Rule.Id.of("1"))
- .name("rule 1")
- .conditionGroup(Rule.ConditionGroup.of(Rule.ConditionCombiner.AND, Rule.Condition.of(FROM, CONTAINS, ALICE.asString())))
- .action(Rule.Action.builder().setAppendInMailboxes(Rule.Action.AppendInMailboxes.withMailboxIds(ImmutableList.of()))
- .setWithKeywords(ImmutableList.of())
- .setForward(Optional.of(forward))
- .build())
- .build());
-
- messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
- .authenticate(ALICE.asString(), PASSWORD)
- .sendMessage(ALICE.asString(), BOB.asString());
-
- Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 2L);
-
- SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
- Mail mail1 = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
- .filter(Throwing.predicate(mail -> mail.getRecipients().contains(BOB.asMailAddress())))
- .findAny().get();
-
- softly.assertThat(mail1.getRecipients()).containsOnly(BOB.asMailAddress());
- }));
- }
-
@Test
void forwardShouldNotKeepACopyWhenKeepACopyIsFalse() throws Exception {
- ImmutableList<MailAddress> forwardedMailAddresses = ImmutableList.of(CEDRIC.asMailAddress());
- Rule.Action.Forward forward = Rule.Action.Forward.of(forwardedMailAddresses, false);
- filteringManagementProbe.defineRulesForUser(BOB,
- Optional.empty(),
- Rule.builder()
- .id(Rule.Id.of("1"))
- .name("rule 1")
- .conditionGroup(Rule.ConditionGroup.of(Rule.ConditionCombiner.AND, Rule.Condition.of(FROM, CONTAINS, ALICE.asString())))
- .action(Rule.Action.builder().setAppendInMailboxes(Rule.Action.AppendInMailboxes.withMailboxIds(ImmutableList.of()))
- .setWithKeywords(ImmutableList.of())
- .setForward(Optional.of(forward))
- .build())
- .build());
+ filteringManagementProbe.defineRulesForUser(BOB, asRule(Forward.to(CEDRIC.asMailAddress()).withoutACopy()));
messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
.authenticate(ALICE.asString(), PASSWORD)
@@ -194,11 +147,7 @@ public class FilterForwardIntegrationTest {
Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
- SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
- softly.assertThat(mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
- .anyMatch(Throwing.predicate(mail -> mail.getRecipients().contains(BOB.asMailAddress())))).isFalse();
- }));
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> softly.assertThat(mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
+ .anyMatch(Throwing.predicate(mail -> mail.getRecipients().contains(BOB.asMailAddress())))).isFalse()));
}
-
-
}
diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardIntegrationTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardIntegrationTest.java
index 316d6de62f..6c6189d715 100644
--- a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardIntegrationTest.java
+++ b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardIntegrationTest.java
@@ -311,7 +311,7 @@ public class ForwardIntegrationTest {
}
@Test
- void forwardShouldSplitLocalCopy() throws Exception {
+ void forwardShouldHandleLocalCopy() throws Exception {
jamesServer.getProbe(DataProbeImpl.class)
.addMapping(MappingSource.fromUser(BOB),
Mapping.forward(DELPHINE.asString()));
@@ -323,19 +323,13 @@ public class ForwardIntegrationTest {
.authenticate(FROM, PASSWORD)
.sendMessage(FROM, BOB.asString());
- Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 2L);
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
Mail forwardedMail = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
- .filter(Throwing.predicate(mail -> mail.getRecipients().contains(DELPHINE.asMailAddress())))
.findAny().get();
- Mail localCopy = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
- .filter(Throwing.predicate(mail -> mail.getRecipients().contains(BOB.asMailAddress())))
- .findAny().get();
- softly.assertThat(forwardedMail.getRecipients()).containsOnly(DELPHINE.asMailAddress());
+ softly.assertThat(forwardedMail.getRecipients()).containsOnly(DELPHINE.asMailAddress(), BOB.asMailAddress());
softly.assertThat(forwardedMail.getMaybeSender().asOptional()).contains(BOB.asMailAddress());
- softly.assertThat(localCopy.getRecipients()).containsOnly(BOB.asMailAddress());
- softly.assertThat(localCopy.getMaybeSender().asOptional()).contains(new MailAddress(FROM));
}));
}
diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardLoopIntegrationTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardLoopIntegrationTest.java
new file mode 100644
index 0000000000..da24d6703e
--- /dev/null
+++ b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/ForwardLoopIntegrationTest.java
@@ -0,0 +1,238 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.jmap.api.filtering.Rule.Condition.Comparator.NOT_CONTAINS;
+import static org.apache.james.jmap.api.filtering.Rule.Condition.Field.FROM;
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+
+import java.io.File;
+import java.util.List;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.filtering.Rule;
+import org.apache.james.jmap.api.filtering.Rule.Action;
+import org.apache.james.jmap.api.filtering.Rule.Action.Forward;
+import org.apache.james.jmap.mailet.filter.JMAPFiltering;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.mailrepository.api.MailRepositoryUrl;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.rrt.lib.Mapping;
+import org.apache.james.rrt.lib.MappingSource;
+import org.apache.james.transport.mailets.RecipientRewriteTable;
+import org.apache.james.transport.mailets.ToRepository;
+import org.apache.james.transport.matchers.All;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.FilteringManagementProbeImpl;
+import org.apache.james.utils.GuiceProbe;
+import org.apache.james.utils.MailRepositoryProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.apache.mailet.Mail;
+import org.assertj.core.api.SoftAssertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+import org.testcontainers.shaded.org.awaitility.Awaitility;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.collect.ImmutableList;
+import com.google.inject.multibindings.Multibinder;
+
+public class ForwardLoopIntegrationTest {
+
+ private static final Username SENDER = Username.of("sender@" + DEFAULT_DOMAIN);
+ private static final Username ALICE = Username.of("alice@" + DEFAULT_DOMAIN);
+ private static final Username BOB = Username.of("bob@" + DEFAULT_DOMAIN);
+ private static final Username CEDRIC = Username.of("cedric@" + DEFAULT_DOMAIN);
+ private static final MailRepositoryUrl CUSTOM_REPOSITORY = MailRepositoryUrl.from("memory://var/mail/custom/");
+ public static final Rule.ConditionGroup CONDITION_GROUP = Rule.ConditionGroup.of(Rule.ConditionCombiner.AND, Rule.Condition.of(FROM, NOT_CONTAINS, "AAA"));
+
+ private static Rule.Builder asRule(Action.Forward forward) {
+ return Rule.builder()
+ .id(Rule.Id.of("1"))
+ .name("rule 1")
+ .conditionGroup(CONDITION_GROUP)
+ .action(Action.builder().setForward(forward));
+ }
+
+ private TemporaryJamesServer jamesServer;
+ private FilteringManagementProbeImpl filteringManagementProbe;
+ private MailRepositoryProbeImpl mailRepositoryProbe;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+ private DataProbeImpl dataProbe;
+
+ @BeforeEach
+ void setup(@TempDir File temporaryFolder) throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withOverrides(binder -> Multibinder.newSetBinder(binder, GuiceProbe.class).addBinding().to(FilteringManagementProbeImpl.class))
+ .withMailetContainer(MailetContainer.builder()
+ .putProcessor(ProcessorConfiguration.root()
+ .addMailet(MailetConfiguration.builder()
+ .matcher(All.class)
+ .mailet(RecipientRewriteTable.class))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(All.class)
+ .mailet(JMAPFiltering.class))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(All.class)
+ .mailet(ToRepository.class)
+ .addProperty("repositoryPath", CUSTOM_REPOSITORY.asString())))
+ .putProcessor(CommonProcessors.error())
+ .putProcessor(CommonProcessors.transport())
+ .putProcessor(CommonProcessors.bounces()))
+ .build(temporaryFolder);
+
+ jamesServer.start();
+
+ dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+
+ dataProbe.addUser(SENDER.asString(), PASSWORD);
+ dataProbe.addUser(ALICE.asString(), PASSWORD);
+ dataProbe.addUser(BOB.asString(), PASSWORD);
+ dataProbe.addUser(CEDRIC.asString(), PASSWORD);
+
+ mailRepositoryProbe = jamesServer.getProbe(MailRepositoryProbeImpl.class);
+
+ filteringManagementProbe = jamesServer.getProbe(FilteringManagementProbeImpl.class);
+ }
+
+ @AfterEach
+ void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ void filterForwardShouldNotCreateLoopError() throws Exception {
+ filteringManagementProbe.defineRulesForUser(ALICE, asRule(Forward.to(BOB.asMailAddress()).withoutACopy()));
+ filteringManagementProbe.defineRulesForUser(BOB, asRule(Forward.to(CEDRIC.asMailAddress()).withoutACopy()));
+ filteringManagementProbe.defineRulesForUser(CEDRIC, asRule(Forward.to(ALICE.asMailAddress()).withoutACopy()));
+
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(SENDER.asString(), PASSWORD)
+ .sendMessage(SENDER.asString(), ALICE.asString());
+
+ Awaitility.await().until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
+
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+ List<Mail> mails = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
+ .collect(ImmutableList.toImmutableList());
+
+ softly.assertThat(mails.get(0).getRecipients()).containsOnly(CEDRIC.asMailAddress());
+ }));
+ }
+
+ @Test
+ void filterForwardShouldNotCreateLoopErrorAndKeepEmailForUserWhoWantToKeepACopy() throws Exception {
+ filteringManagementProbe.defineRulesForUser(ALICE, asRule(Forward.to(BOB.asMailAddress()).keepACopy()));
+ filteringManagementProbe.defineRulesForUser(BOB, asRule(Forward.to(CEDRIC.asMailAddress()).withoutACopy()));
+ filteringManagementProbe.defineRulesForUser(CEDRIC, asRule(Forward.to(ALICE.asMailAddress()).withoutACopy()));
+
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(SENDER.asString(), PASSWORD)
+ .sendMessage(SENDER.asString(), ALICE.asString());
+
+ Awaitility.await().until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 2L);
+
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+ List<Mail> mailListOne = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY, ALICE.asMailAddress());
+ softly.assertThat(mailListOne.get(0).getRecipients()).containsOnly(ALICE.asMailAddress());
+
+ List<Mail> mailListTwo = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY, CEDRIC.asMailAddress());
+ softly.assertThat(mailListTwo.get(0).getRecipients()).containsOnly(CEDRIC.asMailAddress());
+ }));
+ }
+
+ @Test
+ void regularForwardShouldNotCreateLoopError() throws Exception {
+ dataProbe.addMapping(MappingSource.fromUser(ALICE), Mapping.forward(BOB.asString()));
+ dataProbe.addMapping(MappingSource.fromUser(BOB), Mapping.forward(CEDRIC.asString()));
+ dataProbe.addMapping(MappingSource.fromUser(CEDRIC), Mapping.forward(ALICE.asString()));
+
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(SENDER.asString(), PASSWORD)
+ .sendMessage(SENDER.asString(), ALICE.asString());
+
+ Awaitility.await().until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
+
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+ List<Mail> mails = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
+ .collect(ImmutableList.toImmutableList());
+
+ softly.assertThat(mails.get(0).getRecipients()).containsOnly(CEDRIC.asMailAddress());
+ }));
+ }
+
+ @Test
+ void regularForwardShouldNotCreateLoopErrorAndSendEmailToAppropriateReceivers() throws Exception {
+ dataProbe.addMapping(MappingSource.fromUser(ALICE), Mapping.forward(BOB.asString()));
+ dataProbe.addMapping(MappingSource.fromUser(BOB), Mapping.forward(CEDRIC.asString()));
+ dataProbe.addMapping(MappingSource.fromUser(CEDRIC), Mapping.forward(ALICE.asString()));
+ dataProbe.addMapping(MappingSource.fromUser(CEDRIC), Mapping.forward(SENDER.asString()));
+
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(SENDER.asString(), PASSWORD)
+ .sendMessage(SENDER.asString(), ALICE.asString());
+
+ Awaitility.await().until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
+
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+ List<Mail> mails = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
+ .collect(ImmutableList.toImmutableList());
+
+ softly.assertThat(mails.get(0).getRecipients()).containsOnly(SENDER.asMailAddress());
+ }));
+ }
+
+ @Test
+ void forwardShouldNotCreateLoopErrorWhenFilterForwardAndRegularForwardWorkTogether() throws Exception {
+ filteringManagementProbe.defineRulesForUser(ALICE, asRule(Forward.to(BOB.asMailAddress()).withoutACopy()));
+ dataProbe.addMapping(MappingSource.fromUser(BOB), Mapping.forward(CEDRIC.asString()));
+ filteringManagementProbe.defineRulesForUser(CEDRIC, asRule(Forward.to(ALICE.asMailAddress()).withoutACopy()));
+
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(SENDER.asString(), PASSWORD)
+ .sendMessage(SENDER.asString(), ALICE.asString());
+
+ Awaitility.await().until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ Awaitility.await().until(() -> mailRepositoryProbe.getRepositoryMailCount(CUSTOM_REPOSITORY) == 1L);
+
+ SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+ List<Mail> mails = mailRepositoryProbe.listMails(CUSTOM_REPOSITORY)
+ .collect(ImmutableList.toImmutableList());
+
+ softly.assertThat(mails.get(0).getRecipients()).containsOnly(CEDRIC.asMailAddress());
+ }));
+ }
+}
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
index 7d8c382b91..f57372db3d 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
@@ -52,6 +52,7 @@ import org.apache.james.util.AuditTrail;
import org.apache.james.util.MemoizedSupplier;
import org.apache.mailet.DsnParameters;
import org.apache.mailet.DsnParameters.RecipientDsnParameters;
+import org.apache.mailet.LoopPrevention;
import org.apache.mailet.Mail;
import org.apache.mailet.MailetContext;
import org.apache.mailet.ProcessingState;
@@ -222,65 +223,87 @@ public class RecipientRewriteTableProcessor {
.collect(ImmutableList.toImmutableList()));
}
- static ForwardDecision sendACopy(MailetContext context, MailAddress originalRecipient, List<MailAddress> forwards) {
+ static ForwardDecision sendACopy(MailetContext context,
+ MailAddress originalRecipient,
+ Set<MailAddress> newRecipients) {
return mail -> {
MailImpl copy = MailImpl.duplicate(mail);
+ LoopPrevention.RecordedRecipients recordedRecipients = LoopPrevention.RecordedRecipients.fromMail(mail);
try {
copy.setSender(originalRecipient);
- copy.setRecipients(forwards);
+ copy.setRecipients(newRecipients);
+ recordedRecipients.merge(originalRecipient).recordOn(copy);
+
context.sendMail(copy);
- AuditTrail.entry()
- .protocol("mailetcontainer")
- .action("RecipientRewrite")
- .parameters(Throwing.supplier(() -> ImmutableMap.of("mailId", mail.getName(),
- "mimeMessageId", Optional.ofNullable(mail.getMessage())
- .map(Throwing.function(MimeMessage::getMessageID))
- .orElse(""),
- "sender", mail.getMaybeSender().asString(),
- "forwardedMailId", copy.getName(),
- "forwardedMailSender", originalRecipient.asString(),
- "forwardedMailRecipient", StringUtils.join(forwards))))
- .log("Mail forwarded.");
+ recordInAuditTrail(mail, copy, originalRecipient);
} finally {
LifecycleUtil.dispose(copy);
}
};
}
-
+
+ private static void recordInAuditTrail(Mail mail, MailImpl copy, MailAddress originalRecipient) {
+ AuditTrail.entry()
+ .protocol("mailetcontainer")
+ .action("RecipientRewrite")
+ .parameters(Throwing.supplier(() -> ImmutableMap.of("mailId", mail.getName(),
+ "mimeMessageId", Optional.ofNullable(mail.getMessage())
+ .map(Throwing.function(MimeMessage::getMessageID))
+ .orElse(""),
+ "sender", mail.getMaybeSender().asString(),
+ "forwardedMailId", copy.getName(),
+ "forwardedMailSender", originalRecipient.asString(),
+ "forwardedMailRecipient", StringUtils.join(copy.getRecipients()))))
+ .log("Mail forwarded.");
+ }
+
void apply(Mail mail) throws Exception;
}
- public void processForwards(Mail mail) throws MessagingException {
+ public void processForwards(Mail mail) {
if (rewriteSenderUponForward) {
mail.getRecipients()
.stream()
- .flatMap(Throwing.function(this::processForward))
+ .flatMap(Throwing.function(mailAddress -> processForward(mail, mailAddress)))
.forEach(Throwing.consumer(decision -> decision.apply(mail)));
}
}
- private Stream<ForwardDecision> processForward(MailAddress recipient) throws RecipientRewriteTableException {
- ImmutableSet<Mapping> forwards = virtualTableStore.getStoredMappings(MappingSource.fromMailAddress(recipient))
- .select(Mapping.Type.Forward)
- .asStream()
- .collect(ImmutableSet.toImmutableSet());
-
+ private Stream<ForwardDecision> processForward(Mail mail, MailAddress recipient) throws RecipientRewriteTableException {
+ ImmutableSet<Mapping> forwards = getForwards(recipient);
if (forwards.isEmpty()) {
return Stream.of();
}
- Mapping localCopyMapping = Mapping.forward(recipient.asString());
- boolean localCopy = forwards.contains(localCopyMapping);
+
+ return forwardDecision(mail, forwards, recipient);
+ }
+
+ private Stream<ForwardDecision> forwardDecision(Mail mail, ImmutableSet<Mapping> forwards, MailAddress originalRecipient) {
+ LoopPrevention.RecordedRecipients recordedRecipients = LoopPrevention.RecordedRecipients.fromMail(mail);
+
List<MailAddress> forwardedRecipients = forwards.stream()
- .filter(mapping -> !mapping.equals(localCopyMapping)) // remove the local copy
.flatMap(mapping -> mapping.asMailAddress().stream())
.collect(ImmutableList.toImmutableList());
- if (localCopy) {
- return Stream.of(ForwardDecision.sendACopy(mailetContext, recipient, forwardedRecipients));
+ Set<MailAddress> newRecipients = recordedRecipients.nonRecordedRecipients(forwardedRecipients);
+
+ if (recordedRecipients.getRecipients().contains(originalRecipient)) {
+ return Stream.of();
+ }
+
+ if (!newRecipients.isEmpty()) {
+ return Stream.of(ForwardDecision.sendACopy(mailetContext, originalRecipient, newRecipients),
+ ForwardDecision.removeRecipient(originalRecipient));
}
- return Stream.of(ForwardDecision.removeRecipient(recipient),
- ForwardDecision.sendACopy(mailetContext, recipient, forwardedRecipients));
+ return Stream.empty();
+ }
+
+ private ImmutableSet<Mapping> getForwards(MailAddress recipient) throws RecipientRewriteTableException {
+ return virtualTableStore.getStoredMappings(MappingSource.fromMailAddress(recipient))
+ .select(Mapping.Type.Forward)
+ .asStream()
+ .collect(ImmutableSet.toImmutableSet());
}
private void applyDecisionOnDSNParameters(Mail mail, List<Decision> decisions) {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/mailet/filter/ActionApplier.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/mailet/filter/ActionApplier.java
index 681a673b07..1be918cc68 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/mailet/filter/ActionApplier.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/mailet/filter/ActionApplier.java
@@ -19,8 +19,8 @@
package org.apache.james.jmap.mailet.filter;
-import java.util.List;
import java.util.Optional;
+import java.util.Set;
import java.util.stream.Stream;
import javax.inject.Inject;
@@ -39,6 +39,7 @@ import org.apache.james.mailbox.exception.MailboxNotFoundException;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.server.core.MailImpl;
import org.apache.james.util.AuditTrail;
+import org.apache.mailet.LoopPrevention;
import org.apache.mailet.Mail;
import org.apache.mailet.MailetContext;
import org.apache.mailet.StorageDirective;
@@ -49,6 +50,7 @@ import com.github.fge.lambdas.Throwing;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
public class ActionApplier {
public static final Logger LOGGER = LoggerFactory.getLogger(ActionApplier.class);
@@ -104,42 +106,86 @@ public class ActionApplier {
}
public void apply(Stream<Rule.Action> actions) {
- actions.forEach(Throwing.consumer(this::addStorageDirective));
+ actions.forEach(this::applyAction);
}
- private void addStorageDirective(Rule.Action action) throws MessagingException {
+ private boolean shouldProcessingContinue() {
+ return mail.getRecipients().contains(mailAddress);
+ }
+
+ private void applyAction(Rule.Action action) {
+ applyReject(action);
+ applyForward(action);
+ applyStorageDirective(action);
+ }
+
+ private void applyReject(Rule.Action action) {
if (action.isReject()) {
- mail.setRecipients(mail.getRecipients().stream()
- .filter(recipient -> !recipient.equals(mailAddress))
- .collect(ImmutableList.toImmutableList()));
- return;
+ removeFromRecipients();
}
- if (action.getForward().isPresent()) {
- Rule.Action.Forward forward = action.getForward().get();
- if (!forward.isKeepACopy()) {
- mail.setRecipients(mail.getRecipients().stream()
- .filter(recipient -> !recipient.equals(mailAddress))
- .collect(ImmutableList.toImmutableList()));
- }
- sendACopy(mailetContext, mailAddress, forward.getAddresses());
- return;
+ }
+
+ /**
+ * @return a boolean value to show if a local copy should be kept or not
+ */
+ private void applyForward(Rule.Action action) {
+ action.getForward()
+ .filter(any -> shouldProcessingContinue())
+ .ifPresent(Throwing.consumer(forward -> {
+ LoopPrevention.RecordedRecipients recordedRecipients = LoopPrevention.RecordedRecipients.fromMail(mail);
+
+ if (recordedRecipients.getRecipients().contains(mailAddress)) {
+ // Forward is already processed. Do not do it again.
+ return;
+ }
+
+ Set<MailAddress> newRecipients = getNewRecipients(forward, recordedRecipients);
+ if (!newRecipients.isEmpty()) {
+ removeFromRecipients();
+ sendACopy(mailetContext, mailAddress, recordedRecipients, newRecipients);
+ }
+ }));
+ }
+
+
+ private Set<MailAddress> getNewRecipients(Rule.Action.Forward forward, LoopPrevention.RecordedRecipients recordedRecipients) {
+ ImmutableSet.Builder<MailAddress> newRecipientsBuilder = ImmutableSet.<MailAddress>builder()
+ .addAll(recordedRecipients.nonRecordedRecipients(forward.getAddresses()));
+ if (forward.isKeepACopy()) {
+ newRecipientsBuilder.add(mailAddress);
}
- Optional<ImmutableList<String>> targetMailboxes = Optional.of(action.getAppendInMailboxes().getMailboxIds()
- .stream()
- .flatMap(this::asMailboxName)
- .collect(ImmutableList.toImmutableList()))
- .filter(mailboxes -> !mailboxes.isEmpty());
+ return newRecipientsBuilder.build();
+ }
+
+ private void removeFromRecipients() {
+ mail.setRecipients(mail.getRecipients().stream()
+ .filter(recipient -> !recipient.equals(mailAddress))
+ .collect(ImmutableList.toImmutableList()));
+ }
+
+ private void applyStorageDirective(Rule.Action action) {
+ if (shouldProcessingContinue()) {
+ Optional<ImmutableList<String>> targetMailboxes = computeTargetMailboxes(action);
+
+ StorageDirective.Builder storageDirective = StorageDirective.builder();
+ targetMailboxes.ifPresent(storageDirective::targetFolders);
+ storageDirective
+ .seen(Optional.of(action.isMarkAsSeen()).filter(seen -> seen))
+ .important(Optional.of(action.isMarkAsImportant()).filter(seen -> seen))
+ .keywords(Optional.of(action.getWithKeywords()).filter(c -> !c.isEmpty()))
+ .buildOptional()
+ .map(a -> a.encodeAsAttributes(username))
+ .orElse(Stream.of())
+ .forEach(mail::setAttribute);
+ }
+ }
- StorageDirective.Builder storageDirective = StorageDirective.builder();
- targetMailboxes.ifPresent(storageDirective::targetFolders);
- storageDirective
- .seen(Optional.of(action.isMarkAsSeen()).filter(seen -> seen))
- .important(Optional.of(action.isMarkAsImportant()).filter(seen -> seen))
- .keywords(Optional.of(action.getWithKeywords()).filter(c -> !c.isEmpty()))
- .buildOptional()
- .map(a -> a.encodeAsAttributes(username))
- .orElse(Stream.of())
- .forEach(mail::setAttribute);
+ private Optional<ImmutableList<String>> computeTargetMailboxes(Rule.Action action) {
+ return Optional.of(action.getAppendInMailboxes().getMailboxIds()
+ .stream()
+ .flatMap(this::asMailboxName)
+ .collect(ImmutableList.toImmutableList()))
+ .filter(mailboxes -> !mailboxes.isEmpty());
}
private Stream<String> asMailboxName(String mailboxIdString) {
@@ -159,27 +205,36 @@ public class ActionApplier {
}
}
- private void sendACopy(MailetContext context, MailAddress originalRecipient, List<MailAddress> forwards) throws MessagingException {
+ private void sendACopy(MailetContext context,
+ MailAddress originalRecipient,
+ LoopPrevention.RecordedRecipients recordedRecipients,
+ Set<MailAddress> newRecipients) throws MessagingException {
MailImpl copy = MailImpl.duplicate(mail);
try {
copy.setSender(originalRecipient);
- copy.setRecipients(forwards);
+ copy.setRecipients(newRecipients);
+ recordedRecipients.merge(originalRecipient).recordOn(copy);
+
context.sendMail(copy);
- AuditTrail.entry()
- .protocol("mailetcontainer")
- .action("JMAPFiltering")
- .parameters(Throwing.supplier(() -> ImmutableMap.of("mailId", mail.getName(),
- "mimeMessageId", Optional.ofNullable(mail.getMessage())
- .map(Throwing.function(MimeMessage::getMessageID))
- .orElse(""),
- "sender", mail.getMaybeSender().asString(),
- "forwardedMailId", copy.getName(),
- "forwardedMailSender", originalRecipient.asString(),
- "forwardedMailRecipient", StringUtils.join(forwards))))
- .log("Mail forwarded.");
+ recordInAuditTrail(copy);
} finally {
LifecycleUtil.dispose(copy);
}
}
+
+ private void recordInAuditTrail(MailImpl copy) {
+ AuditTrail.entry()
+ .protocol("mailetcontainer")
+ .action("JMAPFiltering")
+ .parameters(Throwing.supplier(() -> ImmutableMap.of("mailId", mail.getName(),
+ "mimeMessageId", Optional.ofNullable(mail.getMessage())
+ .map(Throwing.function(MimeMessage::getMessageID))
+ .orElse(""),
+ "sender", mail.getMaybeSender().asString(),
+ "forwardedMailId", copy.getName(),
+ "forwardedMailSender", copy.getMaybeSender().asString(),
+ "forwardedMailRecipient", StringUtils.join(copy.getRecipients()))))
+ .log("Mail forwarded.");
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org