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 2019/11/15 02:41:31 UTC
[james-project] 22/30: MAILBOX-392 Strengthen MailboxName validation
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 2f882d4f36a4afa6d32b115c48ed6ca8ff7c3710
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Nov 12 12:22:09 2019 +0700
MAILBOX-392 Strengthen MailboxName validation
---
.../apache/james/mailbox/model/MailboxPath.java | 31 +++++++++++-
.../james/mailbox/model/MailboxPathTest.java | 56 ++++++++++++++++++++++
.../james/mailbox/store/StoreMailboxManager.java | 17 +------
3 files changed, 88 insertions(+), 16 deletions(-)
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 92cbde1..27051e3 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
@@ -26,7 +26,11 @@ import java.util.Optional;
import org.apache.james.mailbox.DefaultMailboxes;
import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.exception.HasEmptyMailboxNameInHierarchyException;
+import org.apache.james.mailbox.exception.MailboxNameException;
+import org.apache.james.mailbox.exception.TooLongMailboxNameException;
+import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
/**
@@ -48,6 +52,11 @@ public class MailboxPath {
return new MailboxPath(MailboxConstants.USER_NAMESPACE, username, mailboxName);
}
+ private static final String INVALID_CHARS = "%*&#";
+ private static final CharMatcher INVALID_CHARS_MATCHER = CharMatcher.anyOf(INVALID_CHARS);
+ // This is the size that all mailbox backend should support
+ public static final int MAX_MAILBOX_NAME_LENGTH = 200;
+
private final String namespace;
private final String user;
private final String name;
@@ -136,7 +145,27 @@ public class MailboxPath {
return this;
}
- public boolean hasEmptyNameInHierarchy(char pathDelimiter) {
+ public void assertAcceptable(char pathDelimiter) throws MailboxNameException {
+ if (hasEmptyNameInHierarchy(pathDelimiter)) {
+ throw new HasEmptyMailboxNameInHierarchyException(asString());
+ }
+ if (nameContainsForbiddenCharacters()) {
+ throw new MailboxNameException(asString() + " contains one of the forbidden characters " + INVALID_CHARS);
+ }
+ if (isMailboxNameTooLong()) {
+ throw new TooLongMailboxNameException("Mailbox name exceeds maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
+ }
+ }
+
+ private boolean nameContainsForbiddenCharacters() {
+ return INVALID_CHARS_MATCHER.matchesAnyOf(name);
+ }
+
+ private boolean isMailboxNameTooLong() {
+ return name.length() > MAX_MAILBOX_NAME_LENGTH;
+ }
+
+ boolean hasEmptyNameInHierarchy(char pathDelimiter) {
String delimiterString = String.valueOf(pathDelimiter);
return this.name.isEmpty()
|| this.name.contains(delimiterString + delimiterString)
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java
index b1817cc..321f58f 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java
@@ -21,9 +21,16 @@
package org.apache.james.mailbox.model;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import org.apache.james.mailbox.DefaultMailboxes;
import org.junit.jupiter.api.Test;
+import org.apache.james.mailbox.exception.HasEmptyMailboxNameInHierarchyException;
+import org.apache.james.mailbox.exception.MailboxNameException;
+import org.apache.james.mailbox.exception.TooLongMailboxNameException;
+
+import com.google.common.base.Strings;
import nl.jqno.equalsverifier.EqualsVerifier;
@@ -182,6 +189,55 @@ class MailboxPathTest {
}
@Test
+ void assertAcceptableShouldThrowOnDoubleSeparator() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", "a..b")
+ .assertAcceptable('.'))
+ .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldThrowOnAnd() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", "a&b")
+ .assertAcceptable('.'))
+ .isInstanceOf(MailboxNameException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldThrowOnSharp() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", "a#b")
+ .assertAcceptable('.'))
+ .isInstanceOf(MailboxNameException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldThrowOnPercent() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", "a%b")
+ .assertAcceptable('.'))
+ .isInstanceOf(MailboxNameException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldThrowOnWildcard() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", "a*b")
+ .assertAcceptable('.'))
+ .isInstanceOf(MailboxNameException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldThrowOnTooLongMailboxName() {
+ assertThatThrownBy(() -> MailboxPath.forUser("user", Strings.repeat("a", 201))
+ .assertAcceptable('.'))
+ .isInstanceOf(TooLongMailboxNameException.class);
+ }
+
+ @Test
+ void assertAcceptableShouldNotThrowOnNotTooLongMailboxName() {
+ assertThatCode(() -> MailboxPath.forUser("user", Strings.repeat("a", 200))
+ .assertAcceptable('.'))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
void isInboxShouldReturnTrueWhenINBOX() {
MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, "user", DefaultMailboxes.INBOX);
assertThat(mailboxPath.isInbox()).isTrue();
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index 86f3ebe..d334fa2 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -42,13 +42,11 @@ import org.apache.james.mailbox.MessageManager;
import org.apache.james.mailbox.MetadataWithMailboxId;
import org.apache.james.mailbox.events.EventBus;
import org.apache.james.mailbox.events.MailboxIdRegistrationKey;
-import org.apache.james.mailbox.exception.HasEmptyMailboxNameInHierarchyException;
import org.apache.james.mailbox.exception.InboxAlreadyCreated;
import org.apache.james.mailbox.exception.InsufficientRightsException;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.exception.MailboxExistsException;
import org.apache.james.mailbox.exception.MailboxNotFoundException;
-import org.apache.james.mailbox.exception.TooLongMailboxNameException;
import org.apache.james.mailbox.extension.PreDeletionHook;
import org.apache.james.mailbox.model.Mailbox;
import org.apache.james.mailbox.model.MailboxACL;
@@ -322,18 +320,12 @@ public class StoreMailboxManager implements MailboxManager {
LOGGER.warn("Ignoring mailbox with empty name");
} else {
MailboxPath sanitizedMailboxPath = mailboxPath.sanitize(mailboxSession.getPathDelimiter());
- if (isMailboxNameTooLong(mailboxPath)) {
- throw new TooLongMailboxNameException("Mailbox name exceed maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
- }
+ sanitizedMailboxPath.assertAcceptable(mailboxSession.getPathDelimiter());
if (mailboxExists(sanitizedMailboxPath, mailboxSession)) {
throw new MailboxExistsException(sanitizedMailboxPath.asString());
}
- if (sanitizedMailboxPath.hasEmptyNameInHierarchy(mailboxSession.getPathDelimiter())) {
- throw new HasEmptyMailboxNameInHierarchyException(sanitizedMailboxPath.asString());
- }
-
List<MailboxId> mailboxIds = createMailboxesForPath(mailboxSession, sanitizedMailboxPath);
if (!mailboxIds.isEmpty()) {
@@ -468,12 +460,7 @@ public class StoreMailboxManager implements MailboxManager {
if (mailboxExists(sanitizedMailboxPath, session)) {
throw new MailboxExistsException(sanitizedMailboxPath.toString());
}
- if (isMailboxNameTooLong(sanitizedMailboxPath)) {
- throw new TooLongMailboxNameException("Mailbox name exceed maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
- }
- if (sanitizedMailboxPath.hasEmptyNameInHierarchy(session.getPathDelimiter())) {
- throw new HasEmptyMailboxNameInHierarchyException(to.asString());
- }
+ sanitizedMailboxPath.assertAcceptable(session.getPathDelimiter());
assertIsOwner(session, from);
MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org