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/10/31 09:48:43 UTC

[james-project] 05/06: JAMES-2936 reject path with empty names in the hierachy when creating mailbox

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 <rk...@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org