You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2020/09/25 01:51:57 UTC
[james-project] 12/13: JAMES-3386 do not allow blank mailbox path
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 9a85562e2506f0710304c876eb8b1917a4faa47f
Author: Rémi Kowalski <rk...@linagora.com>
AuthorDate: Wed Sep 23 10:29:36 2020 +0200
JAMES-3386 do not allow blank mailbox path
---
.../apache/james/mailbox/model/MailboxPath.java | 13 +-
.../james/mailbox/model/MailboxPathTest.java | 54 +++++++
.../contract/MailboxSetMethodContract.scala | 167 +++++++++++++++++++++
.../scala/org/apache/james/jmap/mail/Mailbox.scala | 5 +-
.../apache/james/jmap/model/MailboxFactory.scala | 5 +-
5 files changed, 234 insertions(+), 10 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 3d29587..58044a7 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
@@ -25,6 +25,7 @@ import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
+import org.apache.commons.lang3.StringUtils;
import org.apache.james.core.Username;
import org.apache.james.mailbox.DefaultMailboxes;
import org.apache.james.mailbox.MailboxSession;
@@ -35,7 +36,6 @@ import org.apache.james.mailbox.exception.TooLongMailboxNameException;
import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -114,7 +114,7 @@ public class MailboxPath {
}
public MailboxPath child(String childName, char delimiter) {
- Preconditions.checkArgument(!Strings.isNullOrEmpty(childName), "'childName' should not be null or empty");
+ Preconditions.checkArgument(!StringUtils.isBlank(childName), "'childName' should not be null or empty");
Preconditions.checkArgument(!childName.contains(String.valueOf(delimiter)), "'childName' should not contain delimiter");
return new MailboxPath(namespace, user, name + delimiter + childName);
@@ -194,10 +194,11 @@ public class MailboxPath {
boolean hasEmptyNameInHierarchy(char pathDelimiter) {
String delimiterString = String.valueOf(pathDelimiter);
- return this.name.isEmpty()
- || this.name.contains(delimiterString + delimiterString)
- || this.name.startsWith(delimiterString)
- || this.name.endsWith(delimiterString);
+ String nameWithoutSpaces = name.replaceAll("\\s", "");
+ return StringUtils.isBlank(nameWithoutSpaces)
+ || nameWithoutSpaces.contains(delimiterString + delimiterString)
+ || nameWithoutSpaces.startsWith(delimiterString)
+ || nameWithoutSpaces.endsWith(delimiterString);
}
public String asString() {
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 a4d63c7..a160b3c 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
@@ -69,6 +69,12 @@ class MailboxPathTest {
}
@Test
+ void getNameShouldNoopWhenBlank() {
+ assertThat(MailboxPath.forUser(USER, " ").getName('.'))
+ .isEqualTo(" ");
+ }
+
+ @Test
void getHierarchyLevelsShouldBeOrdered() {
assertThat(MailboxPath.forUser(USER, "inbox.folder.subfolder")
.getHierarchyLevels('.'))
@@ -100,6 +106,13 @@ class MailboxPathTest {
}
@Test
+ void childShouldThrowWhenBlank() {
+ MailboxPath path = MailboxPath.forUser(USER, "folder");
+ assertThatThrownBy(() -> path.child(" ", '.'))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
void childShouldThrowWhenContainsDelimiter() {
MailboxPath path = MailboxPath.forUser(USER, "folder");
assertThatThrownBy(() -> path.child("a.b", '.'))
@@ -123,6 +136,14 @@ class MailboxPathTest {
}
@Test
+ void getHierarchyLevelsShouldReturnPathWhenBlankName() {
+ assertThat(MailboxPath.forUser(USER, " ")
+ .getHierarchyLevels('.'))
+ .containsExactly(
+ MailboxPath.forUser(USER, " "));
+ }
+
+ @Test
void getHierarchyLevelsShouldReturnPathWhenNullName() {
assertThat(MailboxPath.forUser(USER, null)
.getHierarchyLevels('.'))
@@ -208,6 +229,13 @@ class MailboxPathTest {
}
@Test
+ void hasEmptyNameInHierarchyShouldBeTrueIfBlankPath() {
+ assertThat(MailboxPath.forUser(USER, " ")
+ .hasEmptyNameInHierarchy('.'))
+ .isTrue();
+ }
+
+ @Test
void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTwoEmptyNames() {
assertThat(MailboxPath.forUser(USER, ".")
.hasEmptyNameInHierarchy('.'))
@@ -220,6 +248,12 @@ class MailboxPathTest {
.hasEmptyNameInHierarchy('.'))
.isTrue();
}
+ @Test
+ void hasEmptyNameInHierarchyShouldBeTrueIfPathWithABlankNameBetweenTwoNames() {
+ assertThat(MailboxPath.forUser(USER, "a. .b")
+ .hasEmptyNameInHierarchy('.'))
+ .isTrue();
+ }
@Test
void hasEmptyNameInHierarchyShouldBeTrueIfPathWithHeadingEmptyNames() {
@@ -229,6 +263,13 @@ class MailboxPathTest {
}
@Test
+ void hasEmptyNameInHierarchyShouldBeTrueIfPathWithHeadingBlankName() {
+ assertThat(MailboxPath.forUser(USER, " .a")
+ .hasEmptyNameInHierarchy('.'))
+ .isTrue();
+ }
+
+ @Test
void hasEmptyNameInHierarchyShouldBeTrueIfPathWithATrailingEmptyName() {
assertThat(MailboxPath.forUser(USER, "a.")
.hasEmptyNameInHierarchy('.'))
@@ -236,11 +277,24 @@ class MailboxPathTest {
}
@Test
+ void hasEmptyNameInHierarchyShouldBeTrueIfPathWithATrailingBlankName() {
+ assertThat(MailboxPath.forUser(USER, "a. ")
+ .hasEmptyNameInHierarchy('.'))
+ .isTrue();
+ }
+
+ @Test
void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTrailingEmptyNames() {
assertThat(MailboxPath.forUser(USER, "a..")
.hasEmptyNameInHierarchy('.'))
.isTrue();
}
+ @Test
+ void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTrailingBlankNames() {
+ assertThat(MailboxPath.forUser(USER, "a. . ")
+ .hasEmptyNameInHierarchy('.'))
+ .isTrue();
+ }
@Test
void assertAcceptableShouldThrowOnDoubleSeparator() {
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index d87adb0..dca59bd 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -332,6 +332,60 @@ trait MailboxSetMethodContract {
}
@Test
+ def updateShouldFailWhenModifyingNameToBlank(server: GuiceJamesServer): Unit = {
+ val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl])
+ .createMailbox(MailboxPath.forUser(BOB, "mailbox"))
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId.serialize()}": {
+ | "name": " "
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [
+ | ["Mailbox/set", {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notUpdated": {
+ | "${mailboxId.serialize()}": {
+ | "type": "invalidArguments",
+ | "description": "'#private:bob@domain.tld: ' has an empty part within its mailbox name considering . as a delimiter",
+ | "properties":["name"]
+ | }
+ | }
+ | }, "c1"]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
def createShouldFailWhenNameWithDelimiter(): Unit = {
val request =
"""
@@ -658,6 +712,119 @@ trait MailboxSetMethodContract {
}
@Test
+ def mailboxSetShouldReturnNotCreatedWhenNameIsEmpty(): Unit = {
+ val request =
+ """
+ |{
+ | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | [
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "create": {
+ | "C42": {
+ | "name": ""
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}
+ |""".stripMargin
+
+ val response: String =
+ `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [[
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notCreated": {
+ | "C42": {
+ | "type": "invalidArguments",
+ | "description": "'/name' property in mailbox object is not valid: Predicate isEmpty() did not fail."
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}""".stripMargin)
+ }
+
+ @Test
+ def mailboxSetShouldReturnNotCreatedWhenNameIsBlank(): Unit = {
+ val request =
+ """
+ |{
+ | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | [
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "create": {
+ | "C42": {
+ | "name": " "
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}
+ |""".stripMargin
+
+ val response: String =
+ `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [[
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notCreated": {
+ | "C42": {
+ | "type": "invalidArguments",
+ | "description": "'#private:bob@domain.tld: ' has an empty part within its mailbox name considering . as a delimiter",
+ | "properties":["name"]
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}""".stripMargin)
+ }
+
+ @Test
def mailboxSetShouldReturnNotCreatedWhenUnknownParameter(): Unit = {
val request =
"""
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
index 28b094c..e9f0985 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
@@ -29,6 +29,7 @@ import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
import org.apache.james.jmap.model.UnsignedInt.UnsignedInt
import org.apache.james.jmap.model.{CapabilityIdentifier, Properties}
import org.apache.james.mailbox.Role
+import org.apache.james.mailbox.exception.MailboxNameException
import org.apache.james.mailbox.model.MailboxId
case class MayReadItems(value: Boolean) extends AnyVal
@@ -120,9 +121,9 @@ object MailboxName {
type MailboxNameConstraint = NonEmpty
type MailboxName = String Refined MailboxNameConstraint
- def validate(value: String): Either[IllegalArgumentException, MailboxName] =
+ def validate(value: String): Either[MailboxNameException, MailboxName] =
refined.refineV[MailboxNameConstraint](value) match {
- case Left(error) => Left(new IllegalArgumentException(error))
+ case Left(error) => Left(new MailboxNameException(error))
case scala.Right(value) => scala.Right(value)
}
}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
index b18544f..4ed2cfb 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
@@ -24,6 +24,7 @@ import org.apache.james.jmap.mail.MailboxName.MailboxName
import org.apache.james.jmap.mail._
import org.apache.james.jmap.utils.quotas.QuotaLoader
import org.apache.james.mailbox._
+import org.apache.james.mailbox.exception.MailboxNameException
import org.apache.james.mailbox.model.MailboxACL.EntryKey
import org.apache.james.mailbox.model.{MailboxCounters, MailboxId, MailboxMetaData, MailboxPath, MailboxACL => JavaMailboxACL}
import reactor.core.scala.publisher.SMono
@@ -32,12 +33,12 @@ import scala.jdk.CollectionConverters._
import scala.jdk.OptionConverters._
object MailboxValidation {
- private def retrieveMailboxName(mailboxPath: MailboxPath, pathDelimiter: Char): Either[IllegalArgumentException, MailboxName] =
+ private def retrieveMailboxName(mailboxPath: MailboxPath, pathDelimiter: Char): Either[MailboxNameException, MailboxName] =
mailboxPath.getName
.split(pathDelimiter)
.lastOption match {
case Some(name) => MailboxName.validate(name)
- case None => Left(new IllegalArgumentException("No name for the mailbox found"))
+ case None => Left(new MailboxNameException("No name for the mailbox found"))
}
def validate(mailboxPath: MailboxPath,
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org