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 2020/06/04 02:18:01 UTC
[james-project] 02/07: JAMES-3199: add validate mailboxname and
also test at UserMailboxesRoutesTest
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 5b9639171bbf6da99ec7cc7bf9cbb01d4b0393dd
Author: duc91 <du...@gmail.com>
AuthorDate: Tue Jun 2 10:41:09 2020 +0700
JAMES-3199: add validate mailboxname and also test at UserMailboxesRoutesTest
---
.../HasEmptyMailboxNameInHierarchyException.java | 9 --
.../apache/james/mailbox/model/MailboxPath.java | 6 +-
.../james/webadmin/routes/UserMailboxesRoutes.java | 17 +--
.../webadmin/service/UserMailboxesService.java | 15 +--
.../james/webadmin/validation/MailboxName.java | 7 +-
.../webadmin/routes/UserMailboxesRoutesTest.java | 116 ++++++++++++++++-----
6 files changed, 111 insertions(+), 59 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
index 5363c95..fc49e42 100644
--- 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
@@ -20,16 +20,7 @@
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/main/java/org/apache/james/mailbox/model/MailboxPath.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java
index 3c6a8ed..36340ee 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
@@ -159,9 +159,10 @@ public class MailboxPath {
return this;
}
- public void assertAcceptable(char pathDelimiter) throws MailboxNameException {
+ public MailboxPath assertAcceptable(char pathDelimiter) throws MailboxNameException {
if (hasEmptyNameInHierarchy(pathDelimiter)) {
- throw new HasEmptyMailboxNameInHierarchyException(asString());
+ throw new HasEmptyMailboxNameInHierarchyException(
+ 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);
@@ -169,6 +170,7 @@ public class MailboxPath {
if (isMailboxNameTooLong()) {
throw new TooLongMailboxNameException("Mailbox name exceeds maximum size of " + MAX_MAILBOX_NAME_LENGTH + " characters");
}
+ return this;
}
private boolean nameContainsForbiddenCharacters() {
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
index c714376..a18747d 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
@@ -35,6 +35,7 @@ import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import org.apache.james.core.Username;
+import org.apache.james.mailbox.exception.MailboxNameException;
import org.apache.james.mailbox.indexer.ReIndexer;
import org.apache.james.task.TaskManager;
import org.apache.james.webadmin.Constants;
@@ -217,12 +218,12 @@ public class UserMailboxesRoutes implements Routes {
.message("Attempt to delete a mailbox with children")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to delete an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
- .message("Attempt to create an invalid mailbox")
+ .message("Attempt to delete an invalid mailbox")
.cause(e)
.haltError();
}
@@ -291,12 +292,12 @@ public class UserMailboxesRoutes implements Routes {
.message("Invalid get on user mailboxes")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to test existence of an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
- .message("Attempt to create an invalid mailbox")
+ .message("Attempt to test existence of an invalid mailbox")
.cause(e)
.haltError();
}
@@ -330,8 +331,8 @@ public class UserMailboxesRoutes implements Routes {
.message("Invalid get on user mailboxes")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Attempt to create an invalid mailbox");
+ } catch (IllegalArgumentException | MailboxNameException e) {
+ LOGGER.info("Attempt to create an invalid mailbox", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
index 003220f..8f4e8e7 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
@@ -64,9 +64,9 @@ public class UserMailboxesService {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
try {
- mailboxManager.createMailbox(
- MailboxPath.forUser(username, mailboxName.asString()),
- mailboxSession);
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
+ mailboxManager.createMailbox(mailboxPath, mailboxSession);
} catch (MailboxExistsException e) {
LOGGER.info("Attempt to create mailbox {} for user {} that already exists", mailboxName, username);
}
@@ -91,16 +91,17 @@ public class UserMailboxesService {
public boolean testMailboxExists(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
- return Mono.from(mailboxManager.mailboxExists(
- MailboxPath.forUser(username, mailboxName.asString()),
- mailboxSession))
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
+ return Mono.from(mailboxManager.mailboxExists(mailboxPath, mailboxSession))
.block();
}
public void deleteMailbox(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException, MailboxHaveChildrenException {
usernamePreconditions(username);
MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
- MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString());
+ MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
+ .assertAcceptable(mailboxSession.getPathDelimiter());
listChildren(mailboxPath, mailboxSession)
.forEach(Throwing.consumer(path -> deleteMailbox(mailboxSession, path)));
}
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
index fdd079d..35d3844 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
@@ -19,18 +19,15 @@
package org.apache.james.webadmin.validation;
-import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
public class MailboxName {
-
- public static final CharMatcher INVALID_CHARS_MATCHER = CharMatcher.anyOf("%*&#");
private final String mailboxName;
public MailboxName(String mailboxName) {
- Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
- Preconditions.checkArgument(INVALID_CHARS_MATCHER.matchesNoneOf(mailboxName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName), "MailboxName must not be null or empty");
+
this.mailboxName = mailboxName;
}
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 5d21359..35f8374 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
@@ -90,6 +90,8 @@ import reactor.core.publisher.Mono;
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 MailboxPath INBOX = MailboxPath.inbox(USERNAME);
private WebAdminServer webAdminServer;
@@ -203,6 +205,84 @@ class UserMailboxesRoutesTest {
}
@Test
+ void putShouldThrowWhenMailboxNameWithDots() throws Exception {
+ Map<String, Object> errors = when()
+ .put(MAILBOX_NAME_WITH_DOTS)
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:my..MailboxName' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenMailboxNameStartsWithDot() throws Exception {
+ Map<String, Object> errors = when()
+ .put(".startWithDot")
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:.startWithDot' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenMailboxNameEndsWithDots() throws Exception {
+ Map<String, Object> errors = when()
+ .put("endWithDot.")
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
+ .containsEntry("type", "InvalidArgument")
+ .containsEntry("message", "Attempt to create an invalid mailbox")
+ .containsEntry("details", "'#private:username:endWithDot.' has an empty part within its mailbox name considering . as a delimiter");
+ }
+
+ @Test
+ void putShouldThrowWhenInvalidMailboxName() throws Exception {
+ when(usersRepository.contains(USERNAME)).thenReturn(true);
+
+ Map<String, Object> errors = when()
+ .put(INVALID_MAILBOX_NAME)
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.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 %*&#");
+ }
+
+ @Test
void deleteShouldReturnNotFoundWithNonExistingUser() throws Exception {
when(usersRepository.contains(USERNAME)).thenReturn(false);
@@ -224,8 +304,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "*")
.then()
@@ -239,13 +317,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName* contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "*")
.then()
@@ -264,8 +341,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "*")
.then()
@@ -284,8 +359,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "%")
.then()
@@ -299,13 +372,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName% contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "%")
.then()
@@ -324,8 +396,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "%")
.then()
@@ -344,8 +414,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "#")
.then()
@@ -359,13 +427,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName# contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "#")
.then()
@@ -384,8 +451,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "#")
.then()
@@ -404,8 +469,6 @@ class UserMailboxesRoutesTest {
@Test
void getShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.get(MAILBOX_NAME + "&")
.then()
@@ -419,13 +482,12 @@ class UserMailboxesRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
.containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Attempt to create an invalid mailbox");
+ .containsEntry("message", "Attempt to test existence of an invalid mailbox")
+ .containsEntry("details", "#private:username:myMailboxName& contains one of the forbidden characters %*&#");
}
@Test
void putShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "&")
.then()
@@ -444,8 +506,6 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception {
- when(usersRepository.contains(USERNAME)).thenReturn(false);
-
Map<String, Object> errors = when()
.put(MAILBOX_NAME + "&")
.then()
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org