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 ad...@apache.org on 2017/11/16 14:20:00 UTC
[13/18] james-project git commit: JAMES-2220 rework MailImpl mailid
generation to make it pass tests
JAMES-2220 rework MailImpl mailid generation to make it pass tests
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/31707935
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/31707935
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/31707935
Branch: refs/heads/master
Commit: 31707935a2318792b609604cd9e6b40311be40ef
Parents: c80bc05
Author: Matthieu Baechler <ma...@apache.org>
Authored: Tue Nov 14 17:26:04 2017 +0100
Committer: Antoine Duprat <ad...@linagora.com>
Committed: Thu Nov 16 12:30:31 2017 +0100
----------------------------------------------------------------------
.../org/apache/james/server/core/MailImpl.java | 52 +++++++++++---------
.../apache/james/server/core/MailImplTest.java | 3 --
2 files changed, 29 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/31707935/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java
----------------------------------------------------------------------
diff --git a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java
index 4ba435c..c5c7f9b 100644
--- a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java
+++ b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java
@@ -39,6 +39,7 @@ import javax.mail.MessagingException;
import javax.mail.internet.MimeMessage;
import javax.mail.internet.ParseException;
+import org.apache.commons.lang.RandomStringUtils;
import org.apache.james.core.MailAddress;
import org.apache.james.lifecycle.api.Disposable;
import org.apache.james.lifecycle.api.LifecycleUtil;
@@ -51,6 +52,7 @@ import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.primitives.Chars;
/**
* <p>
@@ -82,41 +84,45 @@ public class MailImpl implements Disposable, Mail {
return new MailImpl(mail, deriveNewName(mail.getName()));
}
- private static final java.util.Random random = new java.util.Random(); // Used
- // to
- // generate
- // new
- // mail
- // names
-
/**
* Create a unique new primary key name for the given MailObject.
+ * Detect if this has been called more than 8 times recursively
*
* @param currentName the mail to use as the basis for the new mail name
* @return a new name
*/
@VisibleForTesting static String deriveNewName(String currentName) throws MessagingException {
+ char separator = '!';
+ int loopThreshold = 7;
+ int suffixLength = 9;
+ int suffixMaxLength = loopThreshold * suffixLength;
+ int nameMaxLength = suffixMaxLength + 13;
+
+ detectPossibleLoop(currentName, loopThreshold, separator);
// Checking if the original mail name is too long, perhaps because of a
// loop caused by a configuration error.
// it could cause a "null pointer exception" in AvalonMailRepository
- // much
- // harder to understand.
- if (currentName.length() > 76) {
- int count = 0;
- int index = 0;
- while ((index = currentName.indexOf('!', index + 1)) >= 0) {
- count++;
- }
- // It looks like a configuration loop. It's better to stop.
- if (count > 7) {
- throw new MessagingException("Unable to create a new message name: too long." + " Possible loop in config.xml.");
- } else {
- currentName = currentName.substring(0, 76);
- }
- }
+ // much harder to understand.
+ String newName = currentName + generateRandomSuffix(suffixLength, separator);
+ return stripFirstCharsIfNeeded(nameMaxLength, newName);
+ }
- return currentName + "-!" + random.nextInt(1048576);
+ private static String stripFirstCharsIfNeeded(int nameMaxLength, String newName) {
+ return newName.substring(Math.max(0, newName.length() - nameMaxLength));
+ }
+
+ private static String generateRandomSuffix(int suffixLength, char separator) {
+ return "-" + separator + RandomStringUtils.randomNumeric(suffixLength - 2);
+ }
+
+ private static void detectPossibleLoop(String currentName, int loopThreshold, char separator) throws MessagingException {
+ long occurrences = currentName.chars().filter(c -> Chars.saturatedCast(c) == separator).count();
+
+ // It looks like a configuration loop. It's better to stop.
+ if (occurrences > loopThreshold) {
+ throw new MessagingException("Unable to create a new message name: too long. Possible loop in config.xml.");
+ }
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/31707935/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java
----------------------------------------------------------------------
diff --git a/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java b/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java
index e90e4d0..49ce62a 100644
--- a/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java
+++ b/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java
@@ -191,7 +191,6 @@ public class MailImplTest {
}
@Test
- @Ignore("short value doesn't trigger the assertion")
public void deriveNewNameShouldThrowWhenMoreThan8NestedCallsAndSmallInitialValue() throws MessagingException {
String called6Times = IntStream.range(0, 8)
.mapToObj(String::valueOf)
@@ -200,7 +199,6 @@ public class MailImplTest {
}
@Test
- @Ignore("truncation make impossible to detect 8 calls")
public void deriveNewNameShouldThrowWhenMoreThan8NestedCallsAndLongInitialValue() throws MessagingException {
String called6Times = IntStream.range(0, 8)
.mapToObj(String::valueOf)
@@ -208,7 +206,6 @@ public class MailImplTest {
assertThatThrownBy(() -> MailImpl.deriveNewName(called6Times)).isInstanceOf(MessagingException.class);
}
-
@Test
public void deriveNewNameShouldGenerateNotEqualsCurrentName() throws MessagingException {
assertThat(MailImpl.deriveNewName("current")).isNotEqualTo("current");
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org