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 2022/03/22 02:01:43 UTC
[james-project] 12/29: JAMES-3715 IMAP fix framer issues (IndexOutOfBound on bytebuf when turned on/off)
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 <bt...@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-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org