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/10/30 01:27:14 UTC

[james-project] 01/05: JAMES-3432 Limit size attachment

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 bb1fc014db9f0c29bb5d41b3db7edcef26cabebf
Author: duc91 <vd...@linagora.com>
AuthorDate: Mon Oct 26 18:03:02 2020 +0700

    JAMES-3432 Limit size attachment
---
 .../servers/pages/distributed/configure/jmap.adoc  |  8 +++++
 .../src/main/java/org/apache/james/util/Size.java  |  8 ++++-
 .../jmap/rfc8621/contract/UploadContract.scala     | 16 +++++-----
 .../src/test/resources/jmap.properties             |  3 +-
 server/protocols/jmap-rfc-8621/pom.xml             |  5 ++++
 .../jmap/model/JmapRfc8621Configuration.scala      | 16 ++++++----
 .../apache/james/jmap/routes/UploadRoutes.scala    | 34 ++++++++++++++++------
 src/site/xdoc/server/config-jmap.xml               |  8 +++++
 8 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/docs/modules/servers/pages/distributed/configure/jmap.adoc b/docs/modules/servers/pages/distributed/configure/jmap.adoc
index 0b5cce7..f2b55d0 100644
--- a/docs/modules/servers/pages/distributed/configure/jmap.adoc
+++ b/docs/modules/servers/pages/distributed/configure/jmap.adoc
@@ -29,6 +29,14 @@ This should not be the same keystore than the ones used by TLS based protocols.
 | jwt.publickeypem.url
 | Optional. JWT tokens allow request to bypass authentication
 
+| url.prefix
+| Optional. Configuration urlPrefix for JMAP routes.
+| Default value: http://localhost.
+
+| upload.max.size
+| Optional. Configuration max size Upload in new JMAP-RFC-8621.
+| Default value: 30M. Supported units are B (bytes) K (KB) M (MB) G (GB).
+
 |===
 
 == Wire tapping
diff --git a/server/container/util/src/main/java/org/apache/james/util/Size.java b/server/container/util/src/main/java/org/apache/james/util/Size.java
index 341a0e7..263fb54 100644
--- a/server/container/util/src/main/java/org/apache/james/util/Size.java
+++ b/server/container/util/src/main/java/org/apache/james/util/Size.java
@@ -19,6 +19,7 @@
 
 package org.apache.james.util;
 
+import com.google.common.base.Preconditions;
 import com.google.common.math.LongMath;
 
 /**
@@ -35,7 +36,7 @@ public class Size {
      * supported units : B ( 2^0 ), K ( 2^10 ), M ( 2^20 ), G ( 2^30 )
      * See  RFC822.SIZE
      */
-    private enum Unit {
+    public enum Unit {
         NoUnit,
         B,
         K,
@@ -66,6 +67,11 @@ public class Size {
         return new Size(unit, Long.parseLong(argWithoutUnit));
     }
 
+    public static Size of(Long value, Unit unit) {
+        Preconditions.checkArgument(value >= 0, "Maxsize must be positive");
+        return new Size(unit, value);
+    }
+
     public Unit getUnit() {
         return unit;
     }
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/UploadContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/UploadContract.scala
index 770efcd..fe35d0f 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/UploadContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/UploadContract.scala
@@ -24,21 +24,20 @@ import java.nio.charset.StandardCharsets
 import io.netty.handler.codec.http.HttpHeaderNames.ACCEPT
 import io.restassured.RestAssured.{`given`, requestSpecification}
 import io.restassured.http.ContentType
-import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson
 import org.apache.commons.io.IOUtils
-import org.apache.http.HttpStatus.{SC_CREATED, SC_NOT_FOUND, SC_OK, SC_UNAUTHORIZED}
+import org.apache.http.HttpStatus.{SC_BAD_REQUEST, SC_CREATED, SC_OK, SC_UNAUTHORIZED}
 import org.apache.james.GuiceJamesServer
 import org.apache.james.jmap.http.UserCredential
 import org.apache.james.jmap.rfc8621.contract.Fixture.{ACCEPT_RFC8621_VERSION_HEADER, ACCOUNT_ID, ALICE, ALICE_ACCOUNT_ID, ALICE_PASSWORD, BOB, BOB_PASSWORD, DOMAIN, RFC8621_VERSION_HEADER, _2_DOT_DOMAIN, authScheme, baseRequestSpecBuilder}
 import org.apache.james.jmap.rfc8621.contract.UploadContract.{BIG_INPUT_STREAM, VALID_INPUT_STREAM}
 import org.apache.james.utils.DataProbeImpl
 import org.assertj.core.api.Assertions.assertThat
-import org.junit.jupiter.api.{BeforeEach, Disabled, Test}
+import org.junit.jupiter.api.{BeforeEach, Test}
 import play.api.libs.json.{JsString, Json}
 
 object UploadContract {
-  private val BIG_INPUT_STREAM: InputStream = new ByteArrayInputStream("123456789\r\n".repeat(10025).getBytes)
-  private val VALID_INPUT_STREAM: InputStream = new ByteArrayInputStream("123456789\r\n".repeat(1).getBytes)
+  private val BIG_INPUT_STREAM: InputStream = new ByteArrayInputStream("123456789\r\n".repeat(1024 * 1024 * 4).getBytes)
+  private val VALID_INPUT_STREAM: InputStream = new ByteArrayInputStream("123456789\r\n".repeat(1024 * 1024 * 3).getBytes)
 }
 
 trait UploadContract {
@@ -128,7 +127,6 @@ trait UploadContract {
   }
 
   @Test
-  @Disabled("JAMES-1788 Upload size limitation needs to be contributed")
   def shouldRejectWhenUploadFileTooBig(): Unit = {
     val response: String = `given`
       .basePath("")
@@ -138,13 +136,13 @@ trait UploadContract {
     .when
       .post(s"/upload/$ACCOUNT_ID/")
     .`then`
-      .statusCode(SC_OK)
+      .statusCode(SC_BAD_REQUEST)
       .extract
       .body
       .asString
 
-    assertThatJson(response)
-      .isEqualTo("Should be error")
+    assertThat(response)
+      .contains("Attempt to upload exceed max size")
   }
 
   @Test
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/resources/jmap.properties b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/resources/jmap.properties
index a0da434..0128031 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/resources/jmap.properties
+++ b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/resources/jmap.properties
@@ -1,2 +1,3 @@
 # Configuration urlPrefix for JMAP routes.
-url.prefix=http://domain.com
\ No newline at end of file
+url.prefix=http://domain.com
+upload.max.size=30M
\ No newline at end of file
diff --git a/server/protocols/jmap-rfc-8621/pom.xml b/server/protocols/jmap-rfc-8621/pom.xml
index 520910a..048b51e 100644
--- a/server/protocols/jmap-rfc-8621/pom.xml
+++ b/server/protocols/jmap-rfc-8621/pom.xml
@@ -96,6 +96,11 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>commons-fileupload</groupId>
+            <artifactId>commons-fileupload</artifactId>
+            <version>1.4</version>
+        </dependency>
+        <dependency>
             <groupId>com.github.dpaukov</groupId>
             <artifactId>combinatoricslib3</artifactId>
             <scope>test</scope>
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/JmapRfc8621Configuration.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/JmapRfc8621Configuration.scala
index 8865d05..f54636b 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/JmapRfc8621Configuration.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/JmapRfc8621Configuration.scala
@@ -22,18 +22,24 @@ package org.apache.james.jmap.model
 import java.net.URL
 
 import org.apache.commons.configuration2.Configuration
+import org.apache.james.jmap.model.JmapRfc8621Configuration.UPLOAD_LIMIT_30_MB
+import org.apache.james.util.Size
 
 object JmapRfc8621Configuration {
   val LOCALHOST_URL_PREFIX: String = "http://localhost"
-  private def from(urlPrefixString: String): JmapRfc8621Configuration = JmapRfc8621Configuration(urlPrefixString)
-  var LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = from(LOCALHOST_URL_PREFIX)
+  val UPLOAD_LIMIT_30_MB: Size = Size.of(30, Size.Unit.M)
+  val LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = JmapRfc8621Configuration(LOCALHOST_URL_PREFIX, UPLOAD_LIMIT_30_MB)
   val URL_PREFIX_PROPERTIES: String = "url.prefix"
+  val UPLOAD_LIMIT_PROPERTIES: String = "upload.max.size"
 
-  def from(configuration: Configuration): JmapRfc8621Configuration =
-    JmapRfc8621Configuration(Option(configuration.getString(URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_URL_PREFIX))
+  def from(configuration: Configuration): JmapRfc8621Configuration = {
+    JmapRfc8621Configuration(
+      urlPrefixString = Option(configuration.getString(URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_URL_PREFIX),
+      maxUploadSize = Option(configuration.getString(UPLOAD_LIMIT_PROPERTIES, null)).map(Size.parse).getOrElse(UPLOAD_LIMIT_30_MB))
+  }
 }
 
-case class JmapRfc8621Configuration(urlPrefixString: String) {
+case class JmapRfc8621Configuration(urlPrefixString: String, maxUploadSize: Size = UPLOAD_LIMIT_30_MB) {
   val urlPrefix: URL = new URL(urlPrefixString)
   val apiUrl: URL = new URL(s"$urlPrefixString/jmap")
   val downloadUrl: URL = new URL(urlPrefixString + "/download/$accountId/$blobId/?type=$type&name=$name")
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/UploadRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/UploadRoutes.scala
index 6e37695..452543c 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/UploadRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/UploadRoutes.scala
@@ -19,8 +19,7 @@
 
 package org.apache.james.jmap.routes
 
-import java.io.InputStream
-import java.time.ZonedDateTime
+import java.io.{IOException, InputStream, UncheckedIOException}
 import java.util.stream
 import java.util.stream.Stream
 
@@ -45,10 +44,11 @@ import reactor.netty.http.server.{HttpServerRequest, HttpServerResponse}
 import eu.timepit.refined.auto._
 import eu.timepit.refined.numeric.NonNegative
 import eu.timepit.refined.refineV
+import org.apache.commons.fileupload.util.LimitedInputStream
 import org.apache.james.jmap.exceptions.UnauthorizedException
 import org.apache.james.jmap.json.UploadSerializer
 import org.apache.james.jmap.mail.BlobId
-import org.apache.james.jmap.model.{AccountId, Id}
+import org.apache.james.jmap.model.{AccountId, Id, JmapRfc8621Configuration}
 import org.apache.james.jmap.model.Id.Id
 
 object UploadRoutes {
@@ -60,7 +60,7 @@ object UploadRoutes {
   def sanitizeSize(value: Long): Size = {
     val size: Either[String, Size] = refineV[NonNegative](value)
     size.fold(e => {
-      LOGGER.error(s"Encountered an invalid Email size: $e")
+      LOGGER.error(s"Encountered an invalid upload files size: $e")
       Zero
     },
       refinedValue => refinedValue)
@@ -73,10 +73,12 @@ case class UploadResponse(accountId: AccountId,
                           size: Size)
 
 class UploadRoutes @Inject()(@Named(InjectionKeys.RFC_8621) val authenticator: Authenticator,
+                             val configuration: JmapRfc8621Configuration,
                              val attachmentManager: AttachmentManager,
                              val serializer: UploadSerializer) extends JMAPRoutes {
 
   class CancelledUploadException extends RuntimeException
+  class MaxFileSizeUploadException extends RuntimeException
 
   private val accountIdParam: String = "accountId"
   private val uploadURI = s"/upload/{$accountIdParam}/"
@@ -105,6 +107,9 @@ class UploadRoutes @Inject()(@Named(InjectionKeys.RFC_8621) val authenticator: A
     }
   }
 
+  private def handleUncheckedIOException(response: HttpServerResponse, e: UncheckedIOException) =
+    response.status(BAD_REQUEST).sendString(SMono.just(e.getMessage))
+
   def post(request: HttpServerRequest, response: HttpServerResponse, contentType: ContentType, session: MailboxSession): SMono[Void] = {
     Id.validate(request.param(accountIdParam)) match {
       case Right(id: Id) => {
@@ -125,12 +130,23 @@ class UploadRoutes @Inject()(@Named(InjectionKeys.RFC_8621) val authenticator: A
     }
   }
 
-  def handle(accountId: AccountId, contentType: ContentType, content: InputStream, mailboxSession: MailboxSession, response: HttpServerResponse): SMono[Void] =
-    uploadContent(accountId, contentType, content, mailboxSession)
+  def handle(accountId: AccountId, contentType: ContentType, content: InputStream, mailboxSession: MailboxSession, response: HttpServerResponse): SMono[Void] = {
+    val maxSize: Long = configuration.maxUploadSize.asBytes()
+
+    SMono.fromCallable(() => new LimitedInputStream(content, maxSize) {
+      override def raiseError(max: Long, count: Long): Unit = if (count > max) {
+        throw new IOException("Attempt to upload exceed max size")
+      }})
+      .flatMap(uploadContent(accountId, contentType, _, mailboxSession))
       .flatMap(uploadResponse => SMono.fromPublisher(response
-            .header(CONTENT_TYPE, uploadResponse.`type`.asString())
-            .status(CREATED)
-            .sendString(SMono.just(serializer.serialize(uploadResponse).toString()))))
+              .header(CONTENT_TYPE, uploadResponse.`type`.asString())
+              .status(CREATED)
+              .sendString(SMono.just(serializer.serialize(uploadResponse).toString()))))
+      .onErrorResume {
+        case e: UncheckedIOException => SMono.fromPublisher(handleUncheckedIOException(response, e))
+      }
+  }
+
 
   def uploadContent(accountId: AccountId, contentType: ContentType, inputStream: InputStream, session: MailboxSession): SMono[UploadResponse] =
     SMono
diff --git a/src/site/xdoc/server/config-jmap.xml b/src/site/xdoc/server/config-jmap.xml
index b1d28ec..ce649ac 100644
--- a/src/site/xdoc/server/config-jmap.xml
+++ b/src/site/xdoc/server/config-jmap.xml
@@ -56,6 +56,14 @@
 
                     <dt><strong>jwt.publickeypem.url</strong></dt>
                     <dd>Optional. JWT tokens allows request to bypass authentication</dd>
+
+                    <dt><strong>url.prefix</strong></dt>
+                    <dd>Optional. Configuration urlPrefix for JMAP routes.</dd>
+                    <dd>Default value: http://localhost.</dd>
+
+                    <dt><strong>upload.max.size</strong></dt>
+                    <dd>Optional. Configuration max size Upload in new JMAP-RFC-8621.</dd>
+                    <dd>Default value: 30M. Supported units are B (bytes) K (KB) M (MB) G (GB).</dd>
                 </dl>
 
             </subsection>


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