linagora / james-project

Mirror of Apache James Project
Apache License 2.0
72 stars 62 forks source link

Configuration for maximal JSON size in JMAP #5074

Closed chibenwa closed 8 months ago

chibenwa commented 8 months ago

By default that is 20MB.

That's a LOT!

I would prefer seeing a smaller size eg 500KB

That's user input, we should definitly consider it as non-safe!

Let's make this configurable?

CF

com.fasterxml.jackson.core.exc.StreamConstraintsException: String length (20054016) exceeds the maximum length (20000000)
    at com.fasterxml.jackson.core.StreamReadConstraints.validateStringLength(StreamReadConstraints.java:324)
    at com.fasterxml.jackson.core.util.ReadConstrainedTextBuffer.validateStringLength(ReadConstrainedTextBuffer.java:27)
    at com.fasterxml.jackson.core.util.TextBuffer.finishCurrentSegment(TextBuffer.java:939)
    at com.fasterxml.jackson.core.json.UTF8StreamJsonParser._finishString2(UTF8StreamJsonParser.java:2584)
    at com.fasterxml.jackson.core.json.UTF8StreamJsonParser._finishAndReturnString(UTF8StreamJsonParser.java:2560)
    at com.fasterxml.jackson.core.json.UTF8StreamJsonParser.getText(UTF8StreamJsonParser.java:335)
    at play.api.libs.json.jackson.JsValueDeserializer.deserialize(JacksonJson.scala:202)
    at play.api.libs.json.jackson.JsValueDeserializer.deserialize(JacksonJson.scala:157)
    at play.api.libs.json.jackson.JsValueDeserializer.deserialize(JacksonJson.scala:152)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
    at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4801)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2974)
    at play.api.libs.json.jackson.JacksonJson.parseJsValue(JacksonJson.scala:310)
    at play.api.libs.json.StaticBinding$.parseJsValue(StaticBinding.scala:21)
    at play.api.libs.json.Json$.parse(Json.scala:175)
    at org.apache.james.jmap.json.ResponseSerializer$.deserializeRequestObject(ResponseSerializer.scala:169)
    at org.apache.james.jmap.routes.JMAPApiRoutes.parseRequestObject(JMAPApiRoutes.scala:85)
    at org.apache.james.jmap.routes.JMAPApiRoutes.$anonfun$requestAsJsonStream$1(JMAPApiRoutes.scala:80)
    at org.apache.james.jmap.routes.JMAPApiRoutes.$anonfun$requestAsJsonStream$1$adapted(JMAPApiRoutes.scala:79)
    at reactor.core.scala.publisher.package$.$anonfun$scalaBiConsumer2JavaBiConsumer$1(package.scala:55)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:113)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:129)
    at reactor.core.publisher.FluxMap$MapConditionalSubscriber.onNext(FluxMap.java:224)
    at reactor.core.publisher.FluxDoFinally$DoFinallySubscriber.onNext(FluxDoFinally.java:113)
    at reactor.core.publisher.FluxHandleFuseable$HandleFuseableSubscriber.onNext(FluxHandleFuseable.java:194)
    at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
    at reactor.core.publisher.Operators$BaseFluxToMonoOperator.completePossiblyEmpty(Operators.java:2097)
    at reactor.core.publisher.MonoCollectList$MonoCollectListSubscriber.onComplete(MonoCollectList.java:118)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onComplete(FluxPeek.java:260)
    at reactor.core.publisher.FluxMap$MapSubscriber.onComplete(FluxMap.java:144)
    at reactor.netty.channel.FluxReceive.onInboundComplete(FluxReceive.java:415)
    at reactor.netty.channel.ChannelOperations.onInboundComplete(ChannelOperations.java:446)
    at reactor.netty.http.server.HttpServerOperations.onInboundNext(HttpServerOperations.java:687)
    at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:114)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at reactor.netty.http.server.HttpTrafficHandler.channelRead(HttpTrafficHandler.java:284)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
    at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
    at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:800)
    at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:509)
    at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:407)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Unknown Source)

Definition of done

chibenwa commented 8 months ago

Opened https://github.com/playframework/play-json/issues/984

It looks like we are going to need to add a safeguard at the HTTP level when receiving the post request!

chibenwa commented 8 months ago

DOD

In jmap.properties

api.request.max.size=5M

And check that prior deserializing here: https://github.com/apache/james-project/blob/05b72736a07e51c0e83350cc918e2f71b00c785b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala#L85

https://github.com/apache/james-project/blob/05b72736a07e51c0e83350cc918e2f71b00c785b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala#L111

This could be done either by checking readable bytes or by using LimitedInputStream

vttranlina commented 8 months ago

it will easily be done by config in api gateway Example: https://docs.nginx.com/nginx-management-suite/acm/how-to/policies/request-body-size-limit/#:~:text=The%20Request%20Body%20Size%20Limit,error%20code%20will%20be%20returned.

chibenwa commented 8 months ago

Ok with me too.

More on APISIX then.

Please go ahead with a patch on tmail-backend helm chart.

vttranlina commented 8 months ago
chibenwa commented 8 months ago

Reviewed. We IMO need 2 settings: one for uploads, one for everyting else...