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 f278151ba938a5e0b81c7671c443bf434e1bca29 Author: Rémi KOWALSKI <[email protected]> AuthorDate: Mon Oct 28 15:20:08 2019 +0100 JAMES-2936 reject path with empty names in the hierachy when creating mailbox --- .../HasEmptyMailboxNameInHierarchyException.java | 35 ++++++++++++ .../apache/james/mailbox/MailboxManagerTest.java | 52 +++++++++++++++++- .../james/mailbox/store/StoreMailboxManager.java | 62 +++++++++++++--------- 3 files changed, 123 insertions(+), 26 deletions(-) diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java b/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java new file mode 100644 index 0000000..5363c95 --- /dev/null +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/exception/HasEmptyMailboxNameInHierarchyException.java @@ -0,0 +1,35 @@ +/**************************************************************** + * 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.mailbox.exception; + +public class HasEmptyMailboxNameInHierarchyException extends MailboxNameException { + public HasEmptyMailboxNameInHierarchyException() { + super(); + } + + public HasEmptyMailboxNameInHierarchyException(String message) { + super(message); + } + + public HasEmptyMailboxNameInHierarchyException(String message, Throwable cause) { + super(message, cause); + } + +} diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java index 0409ccc..138422c 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java @@ -48,6 +48,7 @@ import org.apache.james.mailbox.events.MailboxIdRegistrationKey; import org.apache.james.mailbox.events.MailboxListener; import org.apache.james.mailbox.events.MessageMoveEvent; import org.apache.james.mailbox.exception.AnnotationException; +import org.apache.james.mailbox.exception.HasEmptyMailboxNameInHierarchyException; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.exception.TooLongMailboxNameException; import org.apache.james.mailbox.extension.PreDeletionHook; @@ -82,7 +83,6 @@ import org.mockito.ArgumentCaptor; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; - import reactor.core.publisher.Mono; /** @@ -211,6 +211,56 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { assertThatThrownBy(() -> mailboxManager.renameMailbox(originPath, MailboxPath.forUser(USER_1, mailboxName), session)) .isInstanceOf(TooLongMailboxNameException.class); } + + @Test + void creatingMailboxShouldNotThrowWhenNameWithoutEmptyHierarchicalLevel() throws Exception { + MailboxSession session = mailboxManager.createSystemSession(USER_1); + + String mailboxName = "a.b.c"; + + assertThatCode(() -> mailboxManager.createMailbox(MailboxPath.forUser(USER_1, mailboxName), session)).doesNotThrowAnyException(); + } + + @Test + void creatingMailboxShouldNotThrowWhenNameWithASingleToBeNormalizedTrailingDelimiter() throws Exception { + MailboxSession session = mailboxManager.createSystemSession(USER_1); + + String mailboxName = "a.b."; + + assertThatCode(() -> mailboxManager.createMailbox(MailboxPath.forUser(USER_1, mailboxName), session)) + .doesNotThrowAnyException(); + } + + @Test + void creatingMailboxShouldThrowWhenNameWithMoreThanOneTrailingDelimiter() throws Exception { + MailboxSession session = mailboxManager.createSystemSession(USER_1); + + String mailboxName = "a.."; + + assertThatThrownBy(() -> mailboxManager.createMailbox(MailboxPath.forUser(USER_1, mailboxName), session)) + .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class); + } + + @Test + void creatingMailboxShouldThrowWhenNameWithHeadingDelimiter() throws Exception { + MailboxSession session = mailboxManager.createSystemSession(USER_1); + + String mailboxName = ".a"; + + assertThatThrownBy(() -> mailboxManager.createMailbox(MailboxPath.forUser(USER_1, mailboxName), session)) + .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class); + } + + @Test + void creatingMailboxShouldThrowWhenNameWithEmptyHierarchicalLevel() throws Exception { + MailboxSession session = mailboxManager.createSystemSession(USER_1); + + String mailboxName = "a..b"; + + assertThatThrownBy(() -> mailboxManager.createMailbox(MailboxPath.forUser(USER_1, mailboxName), session)) + .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class); + } + } @Nested 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 901efe6..7944aa2 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 @@ -43,6 +43,7 @@ import org.apache.james.mailbox.MetadataWithMailboxId; import org.apache.james.mailbox.StandardMailboxMetaDataComparator; 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.InsufficientRightsException; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.exception.MailboxExistsException; @@ -333,37 +334,17 @@ public class StoreMailboxManager implements MailboxManager { if (isMailboxNameTooLong(mailboxPath)) { throw new TooLongMailboxNameException("Mailbox name exceed maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters"); } + if (mailboxExists(sanitizedMailboxPath, mailboxSession)) { throw new MailboxExistsException(sanitizedMailboxPath.asString()); } - // Create parents first - // If any creation fails then the mailbox will not be created - // TODO: transaction - List<MailboxId> mailboxIds = new ArrayList<>(); - for (MailboxPath mailbox : sanitizedMailboxPath.getHierarchyLevels(getDelimiter())) { - locker.executeWithLock(mailboxSession, mailbox, (LockAwareExecution<Void>) () -> { - if (!mailboxExists(mailbox, mailboxSession)) { - Mailbox m = doCreateMailbox(mailbox); - MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); - try { - mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(m)))); - // notify listeners - eventBus.dispatch(EventFactory.mailboxAdded() - .randomEventId() - .mailboxSession(mailboxSession) - .mailbox(m) - .build(), - new MailboxIdRegistrationKey(m.getMailboxId())) - .block(); - } catch (MailboxExistsException e) { - LOGGER.info("{} mailbox was created concurrently", m.generateAssociatedPath()); - } - } - return null; - }, true); + if (sanitizedMailboxPath.hasEmptyNameInHierarchy(mailboxSession.getPathDelimiter())) { + throw new HasEmptyMailboxNameInHierarchyException(sanitizedMailboxPath.asString()); } + List<MailboxId> mailboxIds = createMailboxesForPath(mailboxSession, sanitizedMailboxPath); + if (!mailboxIds.isEmpty()) { return Optional.ofNullable(Iterables.getLast(mailboxIds)); } @@ -371,6 +352,37 @@ public class StoreMailboxManager implements MailboxManager { return Optional.empty(); } + private List<MailboxId> createMailboxesForPath(MailboxSession mailboxSession, MailboxPath sanitizedMailboxPath) throws MailboxException { + // Create parents first + // If any creation fails then the mailbox will not be created + // TODO: transaction + List<MailboxId> mailboxIds = new ArrayList<>(); + for (MailboxPath mailbox : sanitizedMailboxPath.getHierarchyLevels(getDelimiter())) { + locker.executeWithLock(mailboxSession, mailbox, (LockAwareExecution<Void>) () -> { + if (!mailboxExists(mailbox, mailboxSession)) { + Mailbox m = doCreateMailbox(mailbox); + MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); + try { + mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(m)))); + // notify listeners + eventBus.dispatch(EventFactory.mailboxAdded() + .randomEventId() + .mailboxSession(mailboxSession) + .mailbox(m) + .build(), + new MailboxIdRegistrationKey(m.getMailboxId())) + .block(); + } catch (MailboxExistsException e) { + LOGGER.info("{} mailbox was created concurrently", m.generateAssociatedPath()); + } + } + return null; + + }, true); + } + return mailboxIds; + } + private void assertMailboxPathBelongToUser(MailboxSession mailboxSession, MailboxPath mailboxPath) throws MailboxException { if (!mailboxPath.belongsTo(mailboxSession)) { throw new InsufficientRightsException("mailboxPath '" + mailboxPath.asString() + "'" --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
