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/08/27 02:24:39 UTC

[james-project] 07/14: JAMES-3359 Mailbox/set name updates should not contain delimiter

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 ff10686887debdd0936e9001a52215cd8883fced
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Aug 20 14:45:23 2020 +0700

    JAMES-3359 Mailbox/set name updates should not contain delimiter
---
 .../contract/MailboxSetMethodContract.scala        | 53 ++++++++++++++++++++++
 .../org/apache/james/jmap/mail/MailboxSet.scala    | 23 +++++++---
 .../james/jmap/method/MailboxSetMethod.scala       |  2 +-
 3 files changed, 70 insertions(+), 8 deletions(-)

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 3d2a337..765675d 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
@@ -4167,6 +4167,59 @@ trait MailboxSetMethodContract {
          |}""".stripMargin)
   }
 
+  @Test
+  def updateShouldFailWhenMailboxNameContainsDelimiter(server: GuiceJamesServer): Unit = {
+    val mailboxId1: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "previousName"))
+    val request =
+      s"""
+         |{
+         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+         |   "methodCalls": [
+         |       ["Mailbox/set",
+         |           {
+         |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "update": {
+         |                    "${mailboxId1.serialize()}": {
+         |                      "/name": "a.b"
+         |                    }
+         |                }
+         |           },
+         |    "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": {
+         |        "${mailboxId1.serialize()}": {
+         |          "type": "invalidArguments",
+         |          "description": "The mailbox 'a.b' contains an illegal character: '.'",
+         |          "properties": ["/name"]
+         |        }
+         |      }
+         |    }, "c1"]
+         |  ]
+         |}""".stripMargin)
+  }
+
   @Disabled("JAMES-3359 The storage layer should rely on the mailbox path and not the name to allow handling of this case")
   @Test
   def updateShouldAllowDifferentIsSubscribedValuesWhenMailboxHaveTheSameName(server: GuiceJamesServer): Unit = {
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index 3bc5fac..73fc849 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -36,8 +36,8 @@ import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.State.State
 import org.apache.james.jmap.model.{AccountId, CapabilityIdentifier}
 import org.apache.james.jmap.routes.ProcessingContext
-import org.apache.james.mailbox.Role
 import org.apache.james.mailbox.model.{MailboxId, MailboxACL => JavaMailboxACL}
+import org.apache.james.mailbox.{MailboxSession, Role}
 import play.api.libs.json.{JsBoolean, JsError, JsNull, JsObject, JsString, JsSuccess, JsValue}
 
 case class MailboxSetRequest(accountId: AccountId,
@@ -91,8 +91,9 @@ case class MailboxPatchObject(value: Map[String, JsValue]) {
   def validate(processingContext: ProcessingContext,
                mailboxIdFactory: MailboxId.Factory,
                serializer: Serializer,
-               capabilities: Set[CapabilityIdentifier]): Either[PatchUpdateValidationException, ValidatedMailboxPatchObject] = {
-    val asUpdatedIterable = updates(serializer, capabilities, processingContext, mailboxIdFactory)
+               capabilities: Set[CapabilityIdentifier],
+               mailboxSession: MailboxSession): Either[PatchUpdateValidationException, ValidatedMailboxPatchObject] = {
+    val asUpdatedIterable = updates(serializer, capabilities, processingContext, mailboxIdFactory, mailboxSession)
 
     val maybeParseException: Option[PatchUpdateValidationException] = asUpdatedIterable
       .flatMap(x => x match {
@@ -145,9 +146,13 @@ case class MailboxPatchObject(value: Map[String, JsValue]) {
         rightsPartialUpdates = partialRightsUpdates)))
   }
 
-  def updates(serializer: Serializer, capabilities: Set[CapabilityIdentifier], processingContext: ProcessingContext, mailboxIdFactory: MailboxId.Factory): Iterable[Either[PatchUpdateValidationException, Update]] = value.map({
+  def updates(serializer: Serializer,
+              capabilities: Set[CapabilityIdentifier],
+              processingContext: ProcessingContext,
+              mailboxIdFactory: MailboxId.Factory,
+              mailboxSession: MailboxSession): Iterable[Either[PatchUpdateValidationException, Update]] = value.map({
     case (property, newValue) => property match {
-      case "/name" => NameUpdate.parse(newValue)
+      case "/name" => NameUpdate.parse(newValue, mailboxSession)
       case "/parentId" => ParentIdUpdate.parse(newValue, processingContext, mailboxIdFactory)
       case "/sharedWith" => SharedWithResetUpdate.parse(serializer, capabilities)(newValue)
       case "/role" => Left(ServerSetPropertyException(MailboxPatchObject.roleProperty))
@@ -256,8 +261,12 @@ object MailboxSetResponse {
 case class MailboxUpdateResponse(value: JsObject)
 
 object NameUpdate {
-  def parse(newValue: JsValue): Either[PatchUpdateValidationException, Update] = newValue match {
-    case JsString(newName) => scala.Right(NameUpdate(newName))
+  def parse(newValue: JsValue, mailboxSession: MailboxSession): Either[PatchUpdateValidationException, Update] = newValue match {
+    case JsString(newName) => if (newName.contains(mailboxSession.getPathDelimiter)) {
+      Left(InvalidUpdateException("/name", s"The mailbox '$newName' contains an illegal character: '${mailboxSession.getPathDelimiter}'"))
+    } else {
+      scala.Right(NameUpdate(newName))
+    }
     case _ => Left(InvalidUpdateException("/name", "Expecting a JSON string as an argument"))
   }
 }
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 2a22946..ba022f4 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -179,7 +179,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
                             unparsedMailboxId: UnparsedMailboxId,
                             patch: MailboxPatchObject,
                             capabilities: Set[CapabilityIdentifier]): SMono[UpdateResult] = {
-    patch.validate(processingContext, mailboxIdFactory, serializer, capabilities)
+    patch.validate(processingContext, mailboxIdFactory, serializer, capabilities, mailboxSession)
       .fold(e => SMono.raiseError(e), validatedPatch =>
         updateMailboxRights(mailboxId, validatedPatch, mailboxSession)
           .`then`(updateSubscription(mailboxId, validatedPatch, mailboxSession))


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