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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
