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 <btell...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org