This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 365dd4a9dc7c6e0034438d64ef7f81a54575760b Author: Benoit Tellier <btell...@linagora.com> AuthorDate: Mon Feb 10 10:44:40 2020 +0700 JAMES-3042 ImapRequestFrameDecoder::newCumulationBuffer should sanitize negative size NotEnoughDataException is using -1 as a "unknown" marker, making this code path potentially buggy. --- .../james/imapserver/netty/ImapRequestFrameDecoder.java | 15 ++++++++------- .../imapserver/netty/ImapRequestFrameDecoderTest.java | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) 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 801f6d7..c8e35de 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 @@ -42,6 +42,8 @@ import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.channel.ChannelStateEvent; import org.jboss.netty.handler.codec.frame.FrameDecoder; +import com.google.common.annotations.VisibleForTesting; + /** * {@link FrameDecoder} which will decode via and {@link ImapDecoder} instance */ @@ -50,7 +52,8 @@ public class ImapRequestFrameDecoder extends FrameDecoder implements NettyConsta private final ImapDecoder decoder; private final int inMemorySizeLimit; private final int literalSizeLimit; - private static final String NEEDED_DATA = "NEEDED_DATA"; + @VisibleForTesting + static final String NEEDED_DATA = "NEEDED_DATA"; private static final String STORED_DATA = "STORED_DATA"; private static final String WRITTEN_DATA = "WRITTEN_DATA"; private static final String OUTPUT_STREAM = "OUTPUT_STREAM"; @@ -227,13 +230,11 @@ public class ImapRequestFrameDecoder extends FrameDecoder implements NettyConsta @SuppressWarnings("unchecked") int size = (Integer) sizeAsObject; - if (inMemorySizeLimit > 0) { - return ChannelBuffers.dynamicBuffer(Math.min(size, inMemorySizeLimit), ctx.getChannel().getConfig().getBufferFactory()); - } else { + if (size > 0) { + int sanitizedInMemorySizeLimit = Math.max(0, inMemorySizeLimit); + int sanitizedSize = Math.min(sanitizedInMemorySizeLimit, size); - if (size > 0) { - return ChannelBuffers.dynamicBuffer(size, ctx.getChannel().getConfig().getBufferFactory()); - } + return ChannelBuffers.dynamicBuffer(sanitizedSize, ctx.getChannel().getConfig().getBufferFactory()); } } return super.newCumulationBuffer(ctx, minimumCapacity); diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java index 1c66c83..8dffa0b 100644 --- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java +++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java @@ -20,6 +20,7 @@ package org.apache.james.imapserver.netty; +import static org.apache.james.imapserver.netty.ImapRequestFrameDecoder.NEEDED_DATA; import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -60,4 +61,20 @@ class ImapRequestFrameDecoderTest { assertThatCode(() -> testee.newCumulationBuffer(channelHandler, 36)) .doesNotThrowAnyException(); } + + @Test + void newCumulationBufferShouldNotThrowOnNegativeSize() { + ChannelHandlerContext channelHandler = mock(ChannelHandlerContext.class); + Channel channel = mock(Channel.class); + ChannelConfig channelConfig = mock(ChannelConfig.class); + + when(channelConfig.getBufferFactory()).thenReturn(mock(ChannelBufferFactory.class)); + when(channelHandler.getChannel()).thenReturn(channel); + when(channel.getConfig()).thenReturn(channelConfig); + + when(channelHandler.getAttachment()).thenReturn(ImmutableMap.<String, Object>of(NEEDED_DATA, -1)); + + assertThatCode(() -> testee.newCumulationBuffer(channelHandler, 36)) + .doesNotThrowAnyException(); + } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org