You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2020/12/28 07:43:45 UTC

[james-project] 13/16: MAILBOX-392 Allow mailbox names to contain `#`

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 8d2ff807be5ae57e993889a4b59e7c559f120a57
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Dec 25 10:16:01 2020 +0700

    MAILBOX-392 Allow mailbox names to contain `#`
    
    We still reject the `#` prefix as it is reserved to denote
    a namespace. However, the use of # within the name does not lead
    to further ambiguities and can be relaxed.
    
    Names containing `#` are allowed by Cyrus, so relaxing this
    conditions helps with data migration from Cyrus servers.
    
    https://tools.ietf.org/html/rfc3501#section-5.1 simply discourages the use of `#` due to conventions but do
    not require it to be banned.
    
    ```
    Two characters, "#" and "&", have meanings by convention, and
    should be avoided except when used in that convention.
    ```
---
 .../pages/distributed/operate/webadmin.adoc        |  3 +-
 .../apache/james/mailbox/model/MailboxPath.java    |  7 ++--
 .../james/mailbox/model/MailboxPathTest.java       | 11 +++++-
 .../apache/james/imap/scripts/ListSpecialChar.test |  1 +
 .../imapmailbox/inmemory/InMemoryListingTest.java  |  1 +
 .../org/apache/james/cli/MailboxManageTest.java    |  2 +-
 .../webadmin/routes/UserMailboxesRoutesTest.java   | 43 ++++++++++++++++++----
 src/site/markdown/server/manage-webadmin.md        |  2 +-
 8 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/docs/modules/servers/pages/distributed/operate/webadmin.adoc b/docs/modules/servers/pages/distributed/operate/webadmin.adoc
index c88a5e5..be53056 100644
--- a/docs/modules/servers/pages/distributed/operate/webadmin.adoc
+++ b/docs/modules/servers/pages/distributed/operate/webadmin.adoc
@@ -1108,8 +1108,7 @@ curl -XPUT http://ip:port/users/{usernameToBeUsed}/mailboxes/{mailboxNameToBeCre
 ....
 
 Resource name `usernameToBeUsed` should be an existing user Resource
-name `mailboxNameToBeCreated` should not be empty, nor contain # % *
-characters.
+name `mailboxNameToBeCreated` should not be empty, nor contain % * characters, nor starting with #.
 
 Response codes:
 
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 58044a7..6a37d22 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
@@ -62,7 +62,7 @@ public class MailboxPath {
         return new MailboxPath(MailboxConstants.USER_NAMESPACE, username, mailboxName);
     }
 
-    private static final String INVALID_CHARS = "%*#";
+    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;
@@ -176,7 +176,7 @@ public class MailboxPath {
                 String.format("'%s' has an empty part within its mailbox name considering %s as a delimiter", asString(), pathDelimiter));
         }
         if (nameContainsForbiddenCharacters()) {
-            throw new MailboxNameException(asString() + " contains one of the forbidden characters " + INVALID_CHARS);
+            throw new MailboxNameException(asString() + " contains one of the forbidden characters " + INVALID_CHARS + " or starts with #");
         }
         if (isMailboxNameTooLong()) {
             throw new TooLongMailboxNameException("Mailbox name exceeds maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
@@ -185,7 +185,8 @@ public class MailboxPath {
     }
 
     private boolean nameContainsForbiddenCharacters() {
-        return INVALID_CHARS_MATCHER.matchesAnyOf(name);
+        return INVALID_CHARS_MATCHER.matchesAnyOf(name)
+            || name.startsWith("#");
     }
 
     private boolean isMailboxNameTooLong() {
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 a160b3c..dd23a21 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
@@ -304,13 +304,20 @@ class MailboxPathTest {
     }
 
     @Test
-    void assertAcceptableShouldThrowOnSharp() {
-        assertThatThrownBy(() -> MailboxPath.forUser(USER, "a#b")
+    void assertAcceptableShouldThrowWhenStartsWithSharp() {
+        assertThatThrownBy(() -> MailboxPath.forUser(USER, "#ab")
                 .assertAcceptable('.'))
             .isInstanceOf(MailboxNameException.class);
     }
 
     @Test
+    void assertAcceptableShouldNotThrowWhenSharpInTheMiddle() {
+        assertThatCode(() -> MailboxPath.forUser(USER, "mailbox #17")
+                .assertAcceptable('.'))
+            .doesNotThrowAnyException();
+    }
+
+    @Test
     void assertAcceptableShouldThrowOnPercent() {
         assertThatThrownBy(() -> MailboxPath.forUser(USER, "a%b")
                 .assertAcceptable('.'))
diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/ListSpecialChar.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/ListSpecialChar.test
index 2600f85..a2c8fc8 100644
--- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/ListSpecialChar.test
+++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/ListSpecialChar.test
@@ -23,5 +23,6 @@ SUB {
 S: \* LIST \(\\HasNoChildren\) \"\.\" \"INBOX\"
 S: \* LIST \(\\HasNoChildren\) \"\.\" \"projects &- abc\"
 S: \* LIST \(\\HasNoChildren\) \"\.\" \"&AOk-valuations\"
+S: \* LIST \(\\HasNoChildren\) \"\.\" \"mailbox #17\"
 }
 S: a3 OK LIST completed.
diff --git a/mpt/impl/imap-mailbox/inmemory/src/test/java/org/apache/james/mpt/imapmailbox/inmemory/InMemoryListingTest.java b/mpt/impl/imap-mailbox/inmemory/src/test/java/org/apache/james/mpt/imapmailbox/inmemory/InMemoryListingTest.java
index 01ff5e7..5635007 100644
--- a/mpt/impl/imap-mailbox/inmemory/src/test/java/org/apache/james/mpt/imapmailbox/inmemory/InMemoryListingTest.java
+++ b/mpt/impl/imap-mailbox/inmemory/src/test/java/org/apache/james/mpt/imapmailbox/inmemory/InMemoryListingTest.java
@@ -49,6 +49,7 @@ public class InMemoryListingTest extends Listing {
     @Test
     public void listShouldUTF7EscapeSpecialChar() throws Exception {
         system.createMailbox(MailboxPath.forUser(USER, "projects & abc"));
+        system.createMailbox(MailboxPath.forUser(USER, "mailbox #17"));
         system.createMailbox(MailboxPath.forUser(USER, "évaluations"));
 
         simpleScriptedTestProtocol
diff --git a/server/protocols/webadmin-cli/src/test/java/org/apache/james/cli/MailboxManageTest.java b/server/protocols/webadmin-cli/src/test/java/org/apache/james/cli/MailboxManageTest.java
index 4835e9a..98dc2ee 100644
--- a/server/protocols/webadmin-cli/src/test/java/org/apache/james/cli/MailboxManageTest.java
+++ b/server/protocols/webadmin-cli/src/test/java/org/apache/james/cli/MailboxManageTest.java
@@ -256,7 +256,7 @@ public class MailboxManageTest {
                 .addUser("hqtran@linagora.com", "123456");
 
         int exitCode = WebAdminCli.executeFluent(new PrintStream(outputStreamCaptor), new PrintStream(errorStreamCaptor),
-                "--url", "http://127.0.0.1:" + port.getValue(), "mailbox", "delete", "hqtran@linagora.com", "IN#BOX");
+                "--url", "http://127.0.0.1:" + port.getValue(), "mailbox", "delete", "hqtran@linagora.com", "#INBOX");
 
         assertThat(exitCode).isEqualTo(1);
         assertThat(errorStreamCaptor.toString()).contains("Attempt to delete an invalid mailbox");
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index fbd28e9..7724fec 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -141,7 +141,7 @@ class UserMailboxesRoutesTest {
     private static final Username USERNAME = Username.of("username");
     private static final String MAILBOX_NAME = "myMailboxName";
     private static final String MAILBOX_NAME_WITH_DOTS = "my..MailboxName";
-    private static final String INVALID_MAILBOX_NAME = "myMailboxName#";
+    private static final String INVALID_MAILBOX_NAME = "#myMailboxName";
     private static final MailboxPath INBOX = MailboxPath.inbox(USERNAME);
     private static final String ERROR_TYPE_NOTFOUND = "notFound";
     
@@ -332,7 +332,7 @@ class UserMailboxesRoutesTest {
                 .containsEntry("statusCode", BAD_REQUEST_400)
                 .containsEntry("type", "InvalidArgument")
                 .containsEntry("message", "Attempt to create an invalid mailbox")
-                .containsEntry("details", "#private:username:myMailboxName# contains one of the forbidden characters %*#");
+                .containsEntry("details", "#private:username:#myMailboxName contains one of the forbidden characters %* or starts with #");
         }
 
         @Test
@@ -372,7 +372,7 @@ class UserMailboxesRoutesTest {
                 .containsEntry("statusCode", BAD_REQUEST_400)
                 .containsEntry("type", "InvalidArgument")
                 .containsEntry("message", "Attempt to test existence of an invalid mailbox")
-                .containsEntry("details", "#private:username:myMailboxName* contains one of the forbidden characters %*#");
+                .containsEntry("details", "#private:username:myMailboxName* contains one of the forbidden characters %* or starts with #");
         }
 
         @Test
@@ -427,7 +427,7 @@ class UserMailboxesRoutesTest {
                 .containsEntry("statusCode", BAD_REQUEST_400)
                 .containsEntry("type", "InvalidArgument")
                 .containsEntry("message", "Attempt to test existence of an invalid mailbox")
-                .containsEntry("details", "#private:username:myMailboxName% contains one of the forbidden characters %*#");
+                .containsEntry("details", "#private:username:myMailboxName% contains one of the forbidden characters %* or starts with #");
         }
 
         @Test
@@ -469,7 +469,7 @@ class UserMailboxesRoutesTest {
         @Test
         void getShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
             Map<String, Object> errors = when()
-                .get(MAILBOX_NAME + "#")
+                .get("#" + MAILBOX_NAME)
             .then()
                 .statusCode(BAD_REQUEST_400)
                 .contentType(JSON)
@@ -482,13 +482,24 @@ class UserMailboxesRoutesTest {
                 .containsEntry("statusCode", BAD_REQUEST_400)
                 .containsEntry("type", "InvalidArgument")
                 .containsEntry("message", "Attempt to test existence of an invalid mailbox")
-                .containsEntry("details", "#private:username:myMailboxName# contains one of the forbidden characters %*#");
+                .containsEntry("details", "#private:username:#myMailboxName contains one of the forbidden characters %* or starts with #");
+        }
+
+        @Test
+        void getShouldReturnOkWhenSharpInTheMiddleOfTheName() throws Exception {
+            with()
+                .put("a#b");
+
+            when()
+                .get("a#b")
+            .then()
+                .statusCode(NO_CONTENT_204);
         }
 
         @Test
         void putShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
             Map<String, Object> errors = when()
-                .put(MAILBOX_NAME + "#")
+                .put("#" + MAILBOX_NAME)
             .then()
                 .statusCode(BAD_REQUEST_400)
                 .contentType(JSON)
@@ -504,9 +515,17 @@ class UserMailboxesRoutesTest {
         }
 
         @Test
+        void putShouldAcceptMailboxNamesContainingSharp() throws Exception {
+            when()
+                .put("a#b")
+            .then()
+                .statusCode(NO_CONTENT_204);
+        }
+
+        @Test
         void deleteShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
             Map<String, Object> errors = when()
-                .put(MAILBOX_NAME + "#")
+                .put("#" + MAILBOX_NAME)
             .then()
                 .statusCode(BAD_REQUEST_400)
                 .contentType(JSON)
@@ -522,6 +541,14 @@ class UserMailboxesRoutesTest {
         }
 
         @Test
+        void deleteShouldAcceptSharpInTheMiddleOfTheName() throws Exception {
+            when()
+                .put("a#b")
+            .then()
+                .statusCode(NO_CONTENT_204);
+        }
+
+        @Test
         void getShouldReturnNotFoundWithAndMailboxName() throws Exception {
             when()
                 .get(MAILBOX_NAME + "&")
diff --git a/src/site/markdown/server/manage-webadmin.md b/src/site/markdown/server/manage-webadmin.md
index cb6c067..a386681 100644
--- a/src/site/markdown/server/manage-webadmin.md
+++ b/src/site/markdown/server/manage-webadmin.md
@@ -959,7 +959,7 @@ curl -XPUT http://ip:port/users/{usernameToBeUsed}/mailboxes/{mailboxNameToBeCre
 ```
 
 Resource name `usernameToBeUsed` should be an existing user
-Resource name `mailboxNameToBeCreated` should not be empty, nor contain # % * characters.
+Resource name `mailboxNameToBeCreated` should not be empty, nor contain `% *` characters, nor starting with `#`.
 
 Response codes:
 


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org