This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 3ad4edcbff097a0e1a9d758b738ea84fd7db4be9
Author: Benoit Tellier <[email protected]>
AuthorDate: Thu Oct 1 09:28:20 2020 +0700

    JAMES-3396 RecipientRewriteTable should reject loop upon AddressMapping 
Forward Group & Alias creation
    
    Heterogeneous mapping types are handled, as well as domain mappings
---
 .../james/rrt/api/LoopDetectedException.java       | 31 ++++++++
 .../rrt/file/XMLRecipientRewriteTableTest.java     | 35 +++++++++
 .../rrt/lib/AbstractRecipientRewriteTable.java     | 23 ++++++
 .../rrt/lib/RecipientRewriteTableContract.java     | 91 ++++++++++++++++++++++
 .../test/resources/cucumber/rewrite_tables.feature |  7 --
 .../james/transport/mailets/AliasMappingTest.java  | 13 ++--
 .../james/transport/mailets/DomainMappingTest.java |  2 +-
 .../james/transport/mailets/GroupMappingTest.java  | 17 ++--
 .../transport/matchers/IsSenderInRRTLoopTest.java  | 11 ++-
 .../james/smtpserver/ValidRcptHandlerTest.java     |  8 +-
 10 files changed, 209 insertions(+), 29 deletions(-)

diff --git 
a/server/data/data-api/src/main/java/org/apache/james/rrt/api/LoopDetectedException.java
 
b/server/data/data-api/src/main/java/org/apache/james/rrt/api/LoopDetectedException.java
new file mode 100644
index 0000000..bf83830
--- /dev/null
+++ 
b/server/data/data-api/src/main/java/org/apache/james/rrt/api/LoopDetectedException.java
@@ -0,0 +1,31 @@
+/****************************************************************
+ * 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.rrt.api;
+
+import org.apache.james.rrt.lib.Mapping;
+import org.apache.james.rrt.lib.MappingSource;
+
+public class LoopDetectedException extends RecipientRewriteTableException {
+    public LoopDetectedException(MappingSource source, Mapping mapping) {
+        super(String.format("Creation of redirection of %s to %s would lead to 
a loop, operation not performed",
+            source.asString(),
+            mapping.asString()));
+    }
+}
diff --git 
a/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java
 
b/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java
index d3bd13a..32bfc8a 100644
--- 
a/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java
+++ 
b/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java
@@ -242,6 +242,41 @@ class XMLRecipientRewriteTableTest implements 
RecipientRewriteTableContract {
 
     @Test
     @Disabled("XMLRecipientRewriteTable is read only")
+    public void addAddressMappingShouldDetectLoops() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void addAliasMappingShouldDetectLoops() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void addForwardMappingShouldDetectLoops() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void addGroupMappingShouldDetectLoops() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void domainMappingShouldBeHandledAsPartOfLoopDetection() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void heterogeneousLoopsShouldBeDetected() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
+    public void longLoopsShouldBeDetected() {
+    }
+
+    @Test
+    @Disabled("XMLRecipientRewriteTable is read only")
     public void getMappingsForTypeShouldReturnSortedStream() {
     }
 }
diff --git 
a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java
 
b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java
index 0488c13..b4b66b8 100644
--- 
a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java
+++ 
b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java
@@ -33,11 +33,13 @@ import 
org.apache.commons.configuration2.HierarchicalConfiguration;
 import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.configuration2.tree.ImmutableNode;
 import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
 import org.apache.james.core.Username;
 import org.apache.james.domainlist.api.DomainList;
 import org.apache.james.domainlist.api.DomainListException;
 import org.apache.james.lifecycle.api.Configurable;
 import org.apache.james.rrt.api.InvalidRegexException;
+import org.apache.james.rrt.api.LoopDetectedException;
 import org.apache.james.rrt.api.MappingAlreadyExistsException;
 import org.apache.james.rrt.api.RecipientRewriteTable;
 import org.apache.james.rrt.api.RecipientRewriteTableConfiguration;
@@ -197,6 +199,7 @@ public abstract class AbstractRecipientRewriteTable 
implements RecipientRewriteT
         checkNotSameSourceAndDestination(source, address);
 
         LOGGER.info("Add address mapping => {} for source: {}", 
mapping.asString(), source.asString());
+        assertNoLoop(source, mapping);
         addMapping(source, mapping);
     }
 
@@ -279,6 +282,7 @@ public abstract class AbstractRecipientRewriteTable 
implements RecipientRewriteT
         checkDomainMappingSourceIsManaged(source);
 
         LOGGER.info("Add forward mapping => {} for source: {}", 
mapping.asString(), source.asString());
+        assertNoLoop(source, mapping);
         addMapping(source, mapping);
     }
 
@@ -301,6 +305,7 @@ public abstract class AbstractRecipientRewriteTable 
implements RecipientRewriteT
         checkDomainMappingSourceIsManaged(source);
 
         LOGGER.info("Add group mapping => {} for source: {}", 
mapping.asString(), source.asString());
+        assertNoLoop(source, mapping);
         addMapping(source, mapping);
     }
 
@@ -324,6 +329,7 @@ public abstract class AbstractRecipientRewriteTable 
implements RecipientRewriteT
         checkDomainMappingSourceIsManaged(source);
 
         LOGGER.info("Add alias source => {} for destination mapping: {}", 
source.asString(), mapping.asString());
+        assertNoLoop(source, mapping);
         addMapping(source, mapping);
     }
 
@@ -379,4 +385,21 @@ public abstract class AbstractRecipientRewriteTable 
implements RecipientRewriteT
             throw new SameSourceAndDestinationException("Source and 
destination can't be the same!");
         }
     }
+
+    private void assertNoLoop(MappingSource source, Mapping mapping) throws 
RecipientRewriteTableException {
+        if (configuration.isRecursive()) {
+            boolean leadsToALoop = mapping.asMailAddress()
+                .map(Throwing.<MailAddress, Mappings>function(
+                    mailAddress -> 
getResolvedMappings(mailAddress.getLocalPart(), mailAddress.getDomain()))
+                    .sneakyThrow())
+                .map(mappings -> mappings.asStream()
+                    .flatMap(aMapping -> aMapping.asMailAddress().stream())
+                    .anyMatch(address -> 
source.asMailAddress().map(address::equals).orElse(false)))
+                .orElse(false);
+
+            if (leadsToALoop) {
+                throw new LoopDetectedException(source, mapping);
+            }
+        }
+    }
 }
diff --git 
a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java
 
b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java
index 6d41abd..8522b52 100644
--- 
a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java
+++ 
b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java
@@ -26,8 +26,10 @@ import java.util.Map;
 
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
 import org.apache.james.domainlist.api.mock.SimpleDomainList;
 import org.apache.james.lifecycle.api.LifecycleUtil;
+import org.apache.james.rrt.api.LoopDetectedException;
 import org.apache.james.rrt.api.RecipientRewriteTable.ErrorMappingException;
 import org.apache.james.rrt.api.RecipientRewriteTableConfiguration;
 import org.apache.james.rrt.api.RecipientRewriteTableException;
@@ -47,6 +49,7 @@ public interface RecipientRewriteTableContract {
     String ADDRESS = "test@localhost2";
     String ADDRESS_2 = "test@james";
     Domain SUPPORTED_DOMAIN = Domain.LOCALHOST;
+    Domain DOMAIN_2 = Domain.of("adomain.tld");
     MappingSource SOURCE = MappingSource.fromUser(USER, SUPPORTED_DOMAIN);
     Domain NOT_SUPPORTED_DOMAIN = Domain.of("notAManagedDomain");
     MappingSource SOURCE_WITH_DOMAIN_NOT_IN_DOMAIN_LIST = 
MappingSource.fromUser(USER, NOT_SUPPORTED_DOMAIN);
@@ -73,6 +76,7 @@ public interface RecipientRewriteTableContract {
 
         SimpleDomainList domainList = new SimpleDomainList();
         domainList.addDomain(SUPPORTED_DOMAIN);
+        domainList.addDomain(DOMAIN_2);
         virtualUserTable().setDomainList(domainList);
     }
 
@@ -633,4 +637,91 @@ public interface RecipientRewriteTableContract {
         assertThatThrownBy(() -> 
virtualUserTable().addAliasMapping(SOURCE_WITH_DOMAIN_NOT_IN_DOMAIN_LIST, 
ADDRESS))
             .isInstanceOf(SourceDomainIsNotInDomainListException.class);
     }
+
+    @Test
+    default void addAliasMappingShouldDetectLoops() throws Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addAliasMapping(SOURCE, address1);
+
+        assertThatThrownBy(() -> virtualUserTable().addAliasMapping(
+            MappingSource.fromMailAddress(new MailAddress(address1)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void addAddressMappingShouldDetectLoops() throws Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addAddressMapping(SOURCE, address1);
+
+        assertThatThrownBy(() -> virtualUserTable().addAddressMapping(
+            MappingSource.fromMailAddress(new MailAddress(address1)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void addGroupMappingShouldDetectLoops() throws Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addGroupMapping(SOURCE, address1);
+
+        assertThatThrownBy(() -> virtualUserTable().addGroupMapping(
+            MappingSource.fromMailAddress(new MailAddress(address1)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void addForwardMappingShouldDetectLoops() throws Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addForwardMapping(SOURCE, address1);
+
+        assertThatThrownBy(() -> virtualUserTable().addForwardMapping(
+            MappingSource.fromMailAddress(new MailAddress(address1)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void heterogeneousLoopsShouldBeDetected() throws Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addForwardMapping(SOURCE, address1);
+
+        assertThatThrownBy(() -> virtualUserTable().addGroupMapping(
+            MappingSource.fromMailAddress(new MailAddress(address1)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void longLoopsShouldBeDetected() throws Exception {
+        String address1 = "alice@localhost";
+        String address2 = "bob@localhost";
+
+        virtualUserTable().addForwardMapping(SOURCE, address1);
+        virtualUserTable().addForwardMapping(MappingSource.parse(address1), 
address2);
+
+        assertThatThrownBy(() -> virtualUserTable().addGroupMapping(
+            MappingSource.fromMailAddress(new MailAddress(address2)),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
+
+    @Test
+    default void domainMappingShouldBeHandledAsPartOfLoopDetection() throws 
Exception {
+        String address1 = "alice@localhost";
+
+        virtualUserTable().addForwardMapping(SOURCE, address1);
+        
virtualUserTable().addDomainMapping(MappingSource.fromDomain(Domain.LOCALHOST), 
DOMAIN_2);
+
+        assertThatThrownBy(() -> virtualUserTable().addGroupMapping(
+            MappingSource.fromMailAddress(new 
MailAddress("[email protected]")),
+            SOURCE.asString()))
+            .isInstanceOf(LoopDetectedException.class);
+    }
 }
diff --git 
a/server/data/data-library/src/test/resources/cucumber/rewrite_tables.feature 
b/server/data/data-library/src/test/resources/cucumber/rewrite_tables.feature
index b96e217..5bdfd4e 100644
--- 
a/server/data/data-library/src/test/resources/cucumber/rewrite_tables.feature
+++ 
b/server/data/data-library/src/test/resources/cucumber/rewrite_tables.feature
@@ -258,13 +258,6 @@ Feature: Rewrite Tables tests
     And store "user4@domain4" address mapping for user "user3" at domain 
"domain3"
     Then mappings for user "user1" at domain "domain1" should contain only 
"user4@domain4"
 
-  Scenario: recursive mapping should throw exception when a loop exists
-    Given recursive mapping is enable
-    And store "user2@domain2" address mapping for user "user1" at domain 
"domain1"
-    And store "user3@domain3" address mapping for user "user2" at domain 
"domain2"
-    And store "user1@domain1" address mapping for user "user3" at domain 
"domain3"
-    Then retrieving mappings for user "user1" at domain "domain1" should raise 
an ErrorMappingException with message "554 Too many mappings to process"
-
   Scenario: recursive mapping should work when a level is removed
     Given recursive mapping is enable
     And store "user2@domain2" address mapping for user "user1" at domain 
"domain1"
diff --git 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/AliasMappingTest.java
 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/AliasMappingTest.java
index c1ee465..2a5666e 100644
--- 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/AliasMappingTest.java
+++ 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/AliasMappingTest.java
@@ -58,6 +58,7 @@ import io.restassured.specification.RequestSpecification;
 
 public class AliasMappingTest {
     private static final String DOMAIN = "domain.tld";
+    private static final String DOMAIN_2 = "domain2.tld";
 
     private static final String BOB_USER = "bob";
     private static final String ALICE_USER = "alice";
@@ -104,6 +105,7 @@ public class AliasMappingTest {
 
         dataProbe = jamesServer.getProbe(DataProbeImpl.class);
         dataProbe.addDomain(DOMAIN);
+        dataProbe.addDomain(DOMAIN_2);
 
         dataProbe.addUser(BOB_ADDRESS, PASSWORD);
         dataProbe.addUser(ALICE_ADDRESS, PASSWORD);
@@ -379,20 +381,17 @@ public class AliasMappingTest {
 
     @Test
     public void messageShouldBeStoredInRepositoryWhenAliasLoopMapping() throws 
Exception {
-        String bobAlias3 = BOB_USER + "-alias3@" + DOMAIN;
+        String bobAlias2 = BOB_USER + "@" + DOMAIN_2;
 
-        webAdminApi.put(AliasRoutes.ROOT_PATH + "/" + BOB_ALIAS_2 + 
"/sources/" + BOB_ALIAS);
-
-        webAdminApi.put(AliasRoutes.ROOT_PATH + "/" + bobAlias3 + "/sources/" 
+ BOB_ALIAS_2);
-
-        webAdminApi.put(AliasRoutes.ROOT_PATH + "/" + BOB_ALIAS + "/sources/" 
+ bobAlias3);
+        webAdminApi.put(AliasRoutes.ROOT_PATH + "/" + BOB_ADDRESS + 
"/sources/" + bobAlias2);
+        webAdminApi.put("/domains/" + DOMAIN_2 + "/aliases/" + DOMAIN);
 
         messageSender.connect(LOCALHOST_IP, 
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
             .sendMessage(FakeMail.builder()
                 .name("name")
                 .mimeMessage(message)
                 .sender(ALICE_ADDRESS)
-                .recipient(BOB_ALIAS));
+                .recipient(BOB_ADDRESS));
 
         awaitAtMostOneMinute.until(
             () -> jamesServer.getProbe(MailRepositoryProbeImpl.class)
diff --git 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/DomainMappingTest.java
 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/DomainMappingTest.java
index 6824150..81ffdc5 100644
--- 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/DomainMappingTest.java
+++ 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/DomainMappingTest.java
@@ -166,8 +166,8 @@ public class DomainMappingTest {
     public void 
mailShouldGoToRRTErrorMailRepositoryUponLoopCombiningDomainAndAlias() throws 
Exception {
         jamesServer.getProbe(DataProbeImpl.class).addUser(BOB_DOMAIN2, 
PASSWORD);
 
-        webAdminApi.put("/domains/" + DOMAIN1 + "/aliases/" + DOMAIN2);
         webAdminApi.put("/address/aliases/" + BOB_DOMAIN2 + "/sources/" + 
BOB_DOMAIN1);
+        webAdminApi.put("/domains/" + DOMAIN1 + "/aliases/" + DOMAIN2);
 
         messageSender.connect(LOCALHOST_IP, 
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
             .sendMessage(FakeMail.builder()
diff --git 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/GroupMappingTest.java
 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/GroupMappingTest.java
index 0776f93..ea0c288 100644
--- 
a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/GroupMappingTest.java
+++ 
b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/GroupMappingTest.java
@@ -303,8 +303,7 @@ public class GroupMappingTest {
     @Test
     public void senderShouldReceiveABounceUponRRTFailure() throws Exception {
         webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN1 + "/" 
+ GROUP_ON_DOMAIN2);
-
-        webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN2 + "/" 
+ GROUP_ON_DOMAIN1);
+        
jamesServer.getProbe(DataProbeImpl.class).addDomainAliasMapping(DOMAIN2, 
DOMAIN1);
 
         messageSender.connect(LOCALHOST_IP, 
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
             .sendMessage(FakeMail.builder()
@@ -321,15 +320,14 @@ public class GroupMappingTest {
 
     @Test
     public void senderShouldNotReceiveABounceUponRRTFailureWhenPartOfTheLoop() 
throws Exception {
-        webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN1 + "/" 
+ SENDER);
-
-        
jamesServer.getProbe(DataProbeImpl.class).addAddressMapping(SENDER_LOCAL_PART, 
DOMAIN1, GROUP_ON_DOMAIN1);
+        webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN1 + "/" 
+ GROUP_ON_DOMAIN2);
+        
jamesServer.getProbe(DataProbeImpl.class).addDomainAliasMapping(DOMAIN2, 
DOMAIN1);
 
         messageSender.connect(LOCALHOST_IP, 
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
             .sendMessage(FakeMail.builder()
                 .name("name")
                 .mimeMessage(message)
-                .sender(SENDER)
+                .sender(GROUP_ON_DOMAIN1)
                 .recipients(GROUP_ON_DOMAIN1, USER_DOMAIN2));
 
         testIMAPClient.connect(LOCALHOST_IP, 
jamesServer.getProbe(ImapGuiceProbe.class).getImapPort())
@@ -340,15 +338,14 @@ public class GroupMappingTest {
 
     @Test
     public void avoidInfiniteBouncingLoopWhenSenderIsPartOfRRTLoop() throws 
Exception {
-        webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN1 + "/" 
+ SENDER);
-
-        
jamesServer.getProbe(DataProbeImpl.class).addAddressMapping(SENDER_LOCAL_PART, 
DOMAIN1, GROUP_ON_DOMAIN1);
+        webAdminApi.put(GroupsRoutes.ROOT_PATH + "/" + GROUP_ON_DOMAIN1 + "/" 
+ GROUP_ON_DOMAIN2);
+        
jamesServer.getProbe(DataProbeImpl.class).addDomainAliasMapping(DOMAIN2, 
DOMAIN1);
 
         messageSender.connect(LOCALHOST_IP, 
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
             .sendMessage(FakeMail.builder()
                 .name("name")
                 .mimeMessage(message)
-                .sender(SENDER)
+                .sender(GROUP_ON_DOMAIN1)
                 .recipients(GROUP_ON_DOMAIN1, USER_DOMAIN2));
 
         awaitAtMostOneMinute.until(
diff --git 
a/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/IsSenderInRRTLoopTest.java
 
b/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/IsSenderInRRTLoopTest.java
index 6eda370..f0ca51d 100644
--- 
a/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/IsSenderInRRTLoopTest.java
+++ 
b/server/mailet/mailets/src/test/java/org/apache/james/transport/matchers/IsSenderInRRTLoopTest.java
@@ -27,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.Collection;
 
+import org.apache.james.core.Domain;
 import org.apache.james.core.MailAddress;
 import org.apache.james.domainlist.api.mock.SimpleDomainList;
 import org.apache.james.rrt.api.RecipientRewriteTable;
@@ -39,6 +40,7 @@ import org.junit.Test;
 
 public class IsSenderInRRTLoopTest {
 
+    public static final Domain DOMAIN = Domain.of("domain.tld");
     private RecipientRewriteTable recipientRewriteTable;
     private IsSenderInRRTLoop testee;
 
@@ -46,6 +48,7 @@ public class IsSenderInRRTLoopTest {
     public void setUp() throws Exception {
         recipientRewriteTable = new MemoryRecipientRewriteTable();
         SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
         domainList.addDomain(JAMES_LOCAL_DOMAIN);
         ((MemoryRecipientRewriteTable) 
recipientRewriteTable).setDomainList(domainList);
         ((MemoryRecipientRewriteTable) 
recipientRewriteTable).setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED);
@@ -88,8 +91,10 @@ public class IsSenderInRRTLoopTest {
 
     @Test
     public void matchShouldReturnRecipientsWhenLoop() throws Exception {
-        
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(SENDER.getLocalPart(),
 SENDER.getDomain()), RECIPIENT1.asString());
+        
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(SENDER.getLocalPart(),
 SENDER.getDomain()),"[email protected]");
         
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(RECIPIENT1.getLocalPart(),
 RECIPIENT1.getDomain()), SENDER.asString());
+        // required overwise the loop is detected upon insertion
+        
recipientRewriteTable.addDomainMapping(MappingSource.fromDomain(Domain.of("domain.tld")),
 Domain.LOCALHOST);
 
         Collection<MailAddress> result = testee.match(FakeMail.builder()
             .name("name")
@@ -102,8 +107,10 @@ public class IsSenderInRRTLoopTest {
 
     @Test
     public void matchShouldReturnEmptyWhenLoopButNoRecipient() throws 
Exception {
-        
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(SENDER.getLocalPart(),
 SENDER.getDomain()), RECIPIENT1.asString());
+        
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(SENDER.getLocalPart(),
 SENDER.getDomain()),"[email protected]");
         
recipientRewriteTable.addAddressMapping(MappingSource.fromUser(RECIPIENT1.getLocalPart(),
 RECIPIENT1.getDomain()), SENDER.asString());
+        // required overwise the loop is detected upon insertion
+        
recipientRewriteTable.addDomainMapping(MappingSource.fromDomain(DOMAIN), 
Domain.LOCALHOST);
 
         Collection<MailAddress> result = testee.match(FakeMail.builder()
             .name("name")
diff --git 
a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java
 
b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java
index 45bc31b..5065bc6 100644
--- 
a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java
+++ 
b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java
@@ -54,6 +54,7 @@ public class ValidRcptHandlerTest {
     private static final String PASSWORD = "xxx";
     private static final boolean RELAYING_ALLOWED = true;
     private static final MaybeSender MAYBE_SENDER = MaybeSender.of(SENDER);
+    public static final Domain DOMAIN_1 = Domain.of("domain.tld");
 
     private ValidRcptHandler handler;
     private MemoryRecipientRewriteTable memoryRecipientRewriteTable;
@@ -70,6 +71,7 @@ public class ValidRcptHandlerTest {
         UsersRepository users = 
MemoryUsersRepository.withoutVirtualHosting(memoryDomainList);
         users.addUser(VALID_USER, PASSWORD);
 
+        memoryDomainList.addDomain(DOMAIN_1);
         memoryRecipientRewriteTable = new MemoryRecipientRewriteTable();
         memoryRecipientRewriteTable.setDomainList(memoryDomainList);
         
memoryRecipientRewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED);
@@ -194,8 +196,10 @@ public class ValidRcptHandlerTest {
 
     @Test
     public void doRcptShouldDenyWhenHasMappingLoop() throws Exception {
-        
memoryRecipientRewriteTable.addAddressMapping(MappingSource.fromUser(USER1, 
Domain.LOCALHOST), USER2 + "@localhost");
-        
memoryRecipientRewriteTable.addAddressMapping(MappingSource.fromUser(USER2, 
Domain.LOCALHOST), USER1 + "@localhost");
+        
memoryRecipientRewriteTable.addAddressMapping(MappingSource.fromUser(USER1, 
Domain.LOCALHOST), USER2 + "@domain.tld");
+        
memoryRecipientRewriteTable.addAddressMapping(MappingSource.fromUser(USER2, 
DOMAIN_1), USER1 + "@domain.tld");
+        // The loop needs to be created by a domain mapping
+        
memoryRecipientRewriteTable.addDomainMapping(MappingSource.fromDomain(DOMAIN_1),
 Domain.LOCALHOST);
 
         SMTPSession session = setupMockedSMTPSession(!RELAYING_ALLOWED);
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to