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/07/30 04:15:14 UTC
[james-project] 01/12: JAMES-2892 Request level error handling
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 62b20ec9676bf37223bc896eb608d20e233b7117
Author: LanKhuat <dl...@linagora.com>
AuthorDate: Fri Jul 17 16:46:04 2020 +0700
JAMES-2892 Request level error handling
---
.../org/apache/james/jmap/json/Serializer.scala | 4 ++
.../org/apache/james/jmap/model/Capability.scala | 2 +
.../apache/james/jmap/model/ProblemDetails.scala | 29 ++++++++
.../james/jmap/model/RequestLevelErrorType.scala | 31 ++++++++
.../org/apache/james/jmap/model/StatusCode.scala | 27 +++++++
.../apache/james/jmap/routes/JMAPApiRoutes.scala | 65 +++++++++++------
.../james/jmap/routes/JMAPApiRoutesTest.scala | 82 ++++++++++++++++++++--
7 files changed, 212 insertions(+), 28 deletions(-)
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
index b31e0b5..d9e234a 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
@@ -236,12 +236,16 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
private implicit def jsErrorWrites: Writes[JsError] = Json.writes[JsError]
+ private implicit val problemDetailsWrites: Writes[ProblemDetails] = Json.writes[ProblemDetails]
+
def serialize(session: Session): JsValue = Json.toJson(session)
def serialize(requestObject: RequestObject): JsValue = Json.toJson(requestObject)
def serialize(responseObject: ResponseObject): JsValue = Json.toJson(responseObject)
+ def serialize(problemDetails: ProblemDetails): JsValue = Json.toJson(problemDetails)
+
def serialize(mailbox: Mailbox)(implicit mailboxWrites: Writes[Mailbox]): JsValue = Json.toJson(mailbox)
def serialize(mailboxGetResponse: MailboxGetResponse)(implicit mailboxWrites: Writes[Mailbox]): JsValue = Json.toJson(mailboxGetResponse)
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Capability.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Capability.scala
index 6fd841c..8eaaefb 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Capability.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Capability.scala
@@ -34,6 +34,8 @@ object CapabilityIdentifier {
val JMAP_MAIL: CapabilityIdentifier = "urn:ietf:params:jmap:mail"
val JAMES_QUOTA: CapabilityIdentifier = "urn:apache:james:params:jmap:mail:quota"
val JAMES_SHARES: CapabilityIdentifier = "urn:apache:james:params:jmap:mail:shares"
+
+ val SUPPORTED_CAPABILITIES: Set[CapabilityIdentifier] = Set(JMAP_CORE, JMAP_MAIL, JAMES_QUOTA, JAMES_SHARES)
}
sealed trait CapabilityProperties
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/ProblemDetails.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/ProblemDetails.scala
new file mode 100644
index 0000000..f617c86
--- /dev/null
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/ProblemDetails.scala
@@ -0,0 +1,29 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+package org.apache.james.jmap.model
+
+import org.apache.james.jmap.model.RequestLevelErrorType.ErrorTypeIdentifier
+import org.apache.james.jmap.model.StatusCode.ErrorStatus
+
+/**
+ * Problem Details for HTTP APIs within the JMAP context
+ * https://tools.ietf.org/html/rfc7807
+ * see https://jmap.io/spec-core.html#errors
+ */
+case class ProblemDetails(`type`: ErrorTypeIdentifier, status: ErrorStatus, limit: Option[String], detail: String)
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestLevelErrorType.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestLevelErrorType.scala
new file mode 100644
index 0000000..5013a73
--- /dev/null
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestLevelErrorType.scala
@@ -0,0 +1,31 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+package org.apache.james.jmap.model
+
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.string.Uri
+import eu.timepit.refined.auto._
+
+object RequestLevelErrorType {
+ type ErrorTypeIdentifier = String Refined Uri
+ val UNKNOWN_CAPABILITY: ErrorTypeIdentifier = "urn:ietf:params:jmap:error:unknownCapability"
+ val NOT_JSON: ErrorTypeIdentifier = "urn:ietf:params:jmap:error:notJSON"
+ val NOT_REQUEST: ErrorTypeIdentifier = "urn:ietf:params:jmap:error:notRequest"
+ val LIMIT: ErrorTypeIdentifier = "urn:ietf:params:jmap:error:limit"
+}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/StatusCode.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/StatusCode.scala
new file mode 100644
index 0000000..4de45e7
--- /dev/null
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/StatusCode.scala
@@ -0,0 +1,27 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.jmap.model
+
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.numeric.Interval.Closed
+
+object StatusCode {
+ type ErrorStatus = Int Refined Closed[100, 599]
+}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
index 0a89a16..45dfda5 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
@@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets
import java.util.stream
import java.util.stream.Stream
+import com.fasterxml.jackson.core.JsonParseException
import eu.timepit.refined.auto._
import io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE
import io.netty.handler.codec.http.HttpMethod
@@ -38,7 +39,7 @@ import org.apache.james.jmap.json.Serializer
import org.apache.james.jmap.method.Method
import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
-import org.apache.james.jmap.model.{Invocation, RequestObject, ResponseObject}
+import org.apache.james.jmap.model._
import org.apache.james.jmap.{Endpoint, JMAPRoute, JMAPRoutes}
import org.apache.james.mailbox.MailboxSession
import org.slf4j.{Logger, LoggerFactory}
@@ -97,26 +98,33 @@ class JMAPApiRoutes (val authenticator: Authenticator,
private def parseRequestObject(inputStream: InputStream): SMono[RequestObject] =
serializer.deserializeRequestObject(inputStream) match {
case JsSuccess(requestObject, _) => SMono.just(requestObject)
- case JsError(_) => SMono.raiseError(new IllegalArgumentException("Invalid RequestObject"))
+ case errors: JsError => SMono.raiseError(new IllegalArgumentException(serializer.serialize(errors).toString()))
}
private def process(requestObject: RequestObject,
httpServerResponse: HttpServerResponse,
- mailboxSession: MailboxSession): SMono[Void] =
- requestObject
- .methodCalls
- .map(invocation => this.processMethodWithMatchName(requestObject.using.toSet, invocation, mailboxSession))
- .foldLeft(SFlux.empty[Invocation]) { (flux: SFlux[Invocation], mono: SMono[Invocation]) => flux.mergeWith(mono) }
- .collectSeq()
- .flatMap((invocations: Seq[Invocation]) =>
- SMono.fromPublisher(httpServerResponse.status(OK)
- .header(CONTENT_TYPE, JSON_CONTENT_TYPE)
- .sendString(
- SMono.fromCallable(() =>
- serializer.serialize(ResponseObject(ResponseObject.SESSION_STATE, invocations)).toString),
- StandardCharsets.UTF_8
- ).`then`())
- )
+ mailboxSession: MailboxSession): SMono[Void] = {
+ val unsupportedCapabilities = requestObject.using.toSet -- CapabilityIdentifier.SUPPORTED_CAPABILITIES
+
+ if (unsupportedCapabilities.nonEmpty) {
+ SMono.raiseError(UnsupportedCapabilitiesException(unsupportedCapabilities))
+ } else {
+ requestObject
+ .methodCalls
+ .map(invocation => this.processMethodWithMatchName(requestObject.using.toSet, invocation, mailboxSession))
+ .foldLeft(SFlux.empty[Invocation]) { (flux: SFlux[Invocation], mono: SMono[Invocation]) => flux.mergeWith(mono) }
+ .collectSeq()
+ .flatMap((invocations: Seq[Invocation]) =>
+ SMono.fromPublisher(httpServerResponse.status(OK)
+ .header(CONTENT_TYPE, JSON_CONTENT_TYPE)
+ .sendString(
+ SMono.fromCallable(() =>
+ serializer.serialize(ResponseObject(ResponseObject.SESSION_STATE, invocations)).toString),
+ StandardCharsets.UTF_8
+ ).`then`())
+ )
+ }
+ }
private def processMethodWithMatchName(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): SMono[Invocation] =
SMono.justOrEmpty(methodsByName.get(invocation.methodName))
@@ -127,11 +135,26 @@ class JMAPApiRoutes (val authenticator: Authenticator,
invocation.methodCallId)))
private def handleError(throwable: Throwable, httpServerResponse: HttpServerResponse): SMono[Void] = throwable match {
- case exception: IllegalArgumentException => SMono.fromPublisher(httpServerResponse.status(SC_BAD_REQUEST)
- .header(CONTENT_TYPE, JSON_CONTENT_TYPE)
- .sendString(SMono.fromCallable(() => exception.getMessage), StandardCharsets.UTF_8)
- .`then`)
+ case exception: IllegalArgumentException => respondDetails(httpServerResponse,
+ ProblemDetails(RequestLevelErrorType.NOT_REQUEST, SC_BAD_REQUEST, None,
+ s"The request was successfully parsed as JSON but did not match the type signature of the Request object: ${exception.getMessage}"))
case exception: UnauthorizedException => SMono(handleAuthenticationFailure(httpServerResponse, JMAPApiRoutes.LOGGER, exception))
+ case exception: JsonParseException => respondDetails(httpServerResponse,
+ ProblemDetails(RequestLevelErrorType.NOT_JSON, SC_BAD_REQUEST, None,
+ s"The content type of the request was not application/json or the request did not parse as I-JSON: ${exception.getMessage}"))
+ case exception: UnsupportedCapabilitiesException => respondDetails(httpServerResponse,
+ ProblemDetails(RequestLevelErrorType.UNKNOWN_CAPABILITY,
+ SC_BAD_REQUEST, None,
+ s"The request used unsupported capabilities: ${exception.capabilities}"))
case _ => SMono.fromPublisher(handleInternalError(httpServerResponse, throwable))
}
+
+ private def respondDetails(httpServerResponse: HttpServerResponse, details: ProblemDetails): SMono[Void] =
+ SMono.fromPublisher(httpServerResponse.status(SC_BAD_REQUEST)
+ .header(CONTENT_TYPE, JSON_CONTENT_TYPE)
+ .sendString(SMono.fromCallable(() => serializer.serialize(details).toString),
+ StandardCharsets.UTF_8)
+ .`then`)
}
+
+case class UnsupportedCapabilitiesException(capabilities: Set[CapabilityIdentifier]) extends RuntimeException
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
index 37ffb9a..05e4029 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
@@ -38,6 +38,7 @@ import org.apache.james.jmap._
import org.apache.james.jmap.http.{Authenticator, BasicAuthenticationStrategy}
import org.apache.james.jmap.json.Serializer
import org.apache.james.jmap.method.{CoreEchoMethod, Method}
+import org.apache.james.jmap.model.RequestLevelErrorType
import org.apache.james.jmap.routes.JMAPApiRoutesTest._
import org.apache.james.mailbox.MailboxManager
import org.apache.james.mailbox.extension.PreDeletionHook
@@ -45,6 +46,7 @@ import org.apache.james.mailbox.inmemory.MemoryMailboxManagerProvider
import org.apache.james.mailbox.model.TestId
import org.apache.james.metrics.tests.RecordingMetricFactory
import org.apache.james.user.memory.MemoryUsersRepository
+import org.hamcrest.Matchers.equalTo
import org.mockito.Mockito.mock
import org.scalatest.BeforeAndAfter
import org.scalatest.flatspec.AnyFlatSpec
@@ -164,6 +166,32 @@ object JMAPApiRoutesTest {
| }
|}
|""".stripMargin
+
+ private val NOT_JSON_REQUEST: String =
+ """
+ |{
+ | "using": [ "urn:ietf:params:jmap:core"],
+ | "methodCalls": {
+ | "arg1": "arg1data",
+ |}
+ |""".stripMargin
+
+ private val UNKNOWN_CAPABILITY_REQUEST: String =
+ """
+ |{
+ | "using": [ "urn:ietf:params:jmap:core1"],
+ | "methodCalls": [
+ | [
+ | "Core/echo",
+ | {
+ | "arg1": "arg1data",
+ | "arg2": "arg2data"
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}
+ |""".stripMargin
}
class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
@@ -199,7 +227,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.headers(headers)
.when()
.get
- .then
+ .`then`
.statusCode(HttpStatus.SC_NOT_FOUND)
}
@@ -214,7 +242,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.headers(headers)
.when()
.post
- .then
+ .`then`
.statusCode(HttpStatus.SC_OK)
}
@@ -230,7 +258,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.body(REQUEST_OBJECT)
.when()
.post()
- .then
+ .`then`
.statusCode(HttpStatus.SC_OK)
.contentType(ContentType.JSON)
.extract()
@@ -253,7 +281,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.body(REQUEST_OBJECT_WITH_UNSUPPORTED_METHOD)
.when()
.post()
- .then
+ .`then`
.statusCode(HttpStatus.SC_OK)
.contentType(ContentType.JSON)
.extract()
@@ -274,7 +302,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.headers(headers)
.when()
.get
- .then
+ .`then`
.statusCode(HttpStatus.SC_NOT_FOUND)
}
@@ -288,7 +316,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.headers(headers)
.when()
.post
- .then
+ .`then`
.statusCode(HttpStatus.SC_NOT_FOUND)
}
@@ -303,7 +331,47 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
.body(WRONG_OBJECT_REQUEST)
.when()
.post
- .then
+ .`then`
+ .statusCode(HttpStatus.SC_BAD_REQUEST)
+ .body("status", equalTo(400))
+ .body("type", equalTo(RequestLevelErrorType.NOT_REQUEST.value))
+ .body("detail", equalTo("The request was successfully parsed as JSON but did not match the type signature of the Request object: {\"errors\":[{\"path\":\"obj.methodCalls\",\"messages\":[\"error.expected.jsarray\"]}]}"))
+ }
+
+ "RFC-8621 version, POST, with not json request body" should "return 400 status" in {
+ val headers: Headers = Headers.headers(
+ new Header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER),
+ new Header("Authorization", s"Basic ${userBase64String}")
+ )
+ RestAssured
+ .`given`()
+ .headers(headers)
+ .body(NOT_JSON_REQUEST)
+ .when()
+ .post
+ .`then`
+ .statusCode(HttpStatus.SC_BAD_REQUEST)
+ .body("status", equalTo(400))
+ .body("type", equalTo(RequestLevelErrorType.NOT_JSON.value))
+ .body("detail", equalTo("The content type of the request was not application/json or the request did not parse as I-JSON: Unexpected character ('}' (code 125)): was expecting double-quote to start field name\n " +
+ "at [Source: (reactor.netty.ByteBufMono$ReleasingInputStream); line: 6, column: 2]"))
+ }
+
+ "RFC-8621 version, POST, with unknown capability" should "return 400 status" in {
+ val headers: Headers = Headers.headers(
+ new Header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER),
+ new Header("Authorization", s"Basic ${userBase64String}")
+ )
+ RestAssured
+ .`given`()
+ .headers(headers)
+ .body(UNKNOWN_CAPABILITY_REQUEST)
+ .when()
+ .post
+ .`then`
.statusCode(HttpStatus.SC_BAD_REQUEST)
+ .body("status", equalTo(400))
+ .body("type", equalTo(RequestLevelErrorType.UNKNOWN_CAPABILITY.value))
+ .body("detail", equalTo("The request used unsupported capabilities: Set(urn:ietf:params:jmap:core1)"))
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org