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 3222c5dcbb2886c5c75cacd29917b8997e32777e Author: Benoit Tellier <btell...@linagora.com> AuthorDate: Wed Mar 9 15:43:31 2022 +0700 JAMES-3715 IMAP fix framer issues (IndexOutOfBound on bytebuf when turned on/off) This execption was raised for commands following a IMAP APPEND (that did turn off and on the framer), Issue was happening all the time following threading model changes. ``` java.lang.IndexOutOfBoundsException: index: 24, length: -15 (expected: range(0, 65536)) at io.netty.buffer.AbstractByteBuf.checkRangeBounds(AbstractByteBuf.java:1390) at io.netty.buffer.AbstractByteBuf.checkIndex0(AbstractByteBuf.java:1397) at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1384) at io.netty.buffer.AbstractByteBuf.forEachByte(AbstractByteBuf.java:1290) at io.netty.handler.codec.LineBasedFrameDecoder.findEndOfLine(LineBasedFrameDecoder.java:169) at io.netty.handler.codec.LineBasedFrameDecoder.decode(LineBasedFrameDecoder.java:99) at org.apache.james.protocols.netty.AllButStartTlsLineBasedChannelHandler.decode(AllButStartTlsLineBasedChannelHandler.java:61) at io.netty.handler.codec.LineBasedFrameDecoder.decode(LineBasedFrameDecoder.java:84) at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:507) at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:446) ... 10 common frames omitted Wrapped by: io.netty.handler.codec.DecoderException: java.lang.IndexOutOfBoundsException: index: 24, length: -15 (expected: range(0, 65536)) at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:477) at io.netty.handler.codec.ByteToMessageDecoder.channelInputClosed(ByteToMessageDecoder.java:404) at io.netty.handler.codec.ByteToMessageDecoder.channelInputClosed(ByteToMessageDecoder.java:371) at io.netty.handler.codec.ByteToMessageDecoder.channelInactive(ByteToMessageDecoder.java:354) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelInactive(AbstractChannelHandlerContext.java:262) at io.netty.channel.AbstractChannelHandlerContext.access$300(AbstractChannelHandlerContext.java:61) at io.netty.channel.AbstractChannelHandlerContext$4.run(AbstractChannelHandlerContext.java:253) at io.netty.util.concurrent.DefaultEventExecutor.run(DefaultEventExecutor.java:66) at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986) at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) at java.base/java.lang.Thread.run(Thread.java:829) ``` --- .../apache/james/imapserver/netty/IMAPServer.java | 3 +- .../imapserver/netty/ImapRequestFrameDecoder.java | 39 +++++++++++++++++----- .../netty/SwitchableLineBasedFrameDecoder.java | 26 --------------- 3 files changed, 32 insertions(+), 36 deletions(-) diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java index 649f4a8..15132e3 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java @@ -250,7 +250,8 @@ public class IMAPServer extends AbstractConfigurableAsyncServer implements ImapC pipeline.addLast(getExecutorGroup(), CHUNK_WRITE_HANDLER, new ChunkedWriteHandler()); - pipeline.addLast(getExecutorGroup(), REQUEST_DECODER, new ImapRequestFrameDecoder(decoder, inMemorySizeLimit, literalSizeLimit)); + pipeline.addLast(getExecutorGroup(), REQUEST_DECODER, new ImapRequestFrameDecoder(decoder, inMemorySizeLimit, + literalSizeLimit, getExecutorGroup(), maxLineLength)); pipeline.addLast(getExecutorGroup(), CORE_HANDLER, createCoreHandler()); } diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java index 485362b..027fd3c 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java @@ -44,8 +44,8 @@ import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; -import io.netty.channel.ChannelPipeline; import io.netty.handler.codec.ByteToMessageDecoder; +import io.netty.util.concurrent.EventExecutorGroup; /** @@ -62,11 +62,16 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net private final int inMemorySizeLimit; private final int literalSizeLimit; private final Deque<ChannelInboundHandlerAdapter> behaviourOverrides = new ConcurrentLinkedDeque<>(); + private final EventExecutorGroup eventExecutors; + private int maxFrameLength; - public ImapRequestFrameDecoder(ImapDecoder decoder, int inMemorySizeLimit, int literalSizeLimit) { + public ImapRequestFrameDecoder(ImapDecoder decoder, int inMemorySizeLimit, int literalSizeLimit, EventExecutorGroup eventExecutors, + int maxFrameLength) { this.decoder = decoder; this.inMemorySizeLimit = inMemorySizeLimit; this.literalSizeLimit = literalSizeLimit; + this.eventExecutors = eventExecutors; + this.maxFrameLength = maxFrameLength; } @Override @@ -180,7 +185,7 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net reader.consumeLine(); } - ((SwitchableLineBasedFrameDecoder) ctx.channel().pipeline().get(FRAMER)).enableFraming(); + enableFraming(ctx); attachment.clear(); out.add(message); @@ -190,15 +195,15 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net int neededData = e.getNeededSize(); // store the needed data size for later usage attachment.put(NEEDED_DATA, neededData); - - final ChannelPipeline pipeline = ctx.channel().pipeline(); - final ChannelHandlerContext framerContext = pipeline.context(FRAMER); // SwitchableDelimiterBasedFrameDecoder added further to JAMES-1436. - final SwitchableLineBasedFrameDecoder framer = (SwitchableLineBasedFrameDecoder) pipeline.get(FRAMER); - + disableFraming(ctx); + if (in.readableBytes() > 0) { + ByteBuf spareBytes = in.retainedDuplicate(); + internalBuffer().clear(); + ctx.fireChannelRead(spareBytes); + } in.resetReaderIndex(); - framer.disableFraming(framerContext); } } else { // The session was null so may be the case because the channel was already closed but there were still bytes in the buffer. @@ -209,6 +214,22 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net } } + public void disableFraming(ChannelHandlerContext ctx) { + ctx.channel().config().setAutoRead(false); + ctx.channel().eventLoop().execute(() -> ctx.channel().pipeline().remove(FRAMER)); + ctx.channel().config().setAutoRead(true); + } + + public void enableFraming(ChannelHandlerContext ctx) { + if (ctx.channel().pipeline().get(FRAMER) == null) { + ctx.channel().config().setAutoRead(false); + ctx.channel().eventLoop().execute(() -> + ctx.channel().pipeline().addBefore(eventExecutors, REQUEST_DECODER, FRAMER, + new SwitchableLineBasedFrameDecoder(ctx.channel().pipeline(), maxFrameLength, false))); + ctx.channel().config().setAutoRead(true); + } + } + @Override public void pushLineHandler(ChannelInboundHandlerAdapter lineHandlerUpstreamHandler) { behaviourOverrides.addFirst(lineHandlerUpstreamHandler); diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineBasedFrameDecoder.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineBasedFrameDecoder.java index 7ad1257..2f4d117 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineBasedFrameDecoder.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineBasedFrameDecoder.java @@ -25,43 +25,17 @@ import org.apache.james.imap.api.ImapConstants; import org.apache.james.protocols.api.CommandDetectionSession; import org.apache.james.protocols.netty.AllButStartTlsLineBasedChannelHandler; -import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; public class SwitchableLineBasedFrameDecoder extends AllButStartTlsLineBasedChannelHandler { public static final String PATTERN = ImapConstants.STARTTLS_COMMAND.getName().toLowerCase(); - private volatile boolean framingEnabled = true; - public SwitchableLineBasedFrameDecoder(ChannelPipeline pipeline, int maxFrameLength, boolean stripDelimiter) { super(pipeline, maxFrameLength, stripDelimiter, PATTERN); } @Override - public synchronized void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { - if (this.framingEnabled) { - super.channelRead(ctx, msg); - } else { - ctx.fireChannelRead(msg); - } - } - - public synchronized void enableFraming() { - this.framingEnabled = true; - } - - public synchronized void disableFraming(ChannelHandlerContext ctx) { - this.framingEnabled = false; - - if (internalBuffer().readableBytes() > 0) { - ByteBuf spareBytes = internalBuffer().retainedDuplicate(); - internalBuffer().clear(); - ctx.fireChannelRead(spareBytes); - } - } - - @Override protected CommandDetectionSession retrieveSession(ChannelHandlerContext ctx) { return ctx.channel().attr(NettyConstants.IMAP_SESSION_ATTRIBUTE_KEY).get(); } --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org