You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2024/06/24 07:57:08 UTC
(james-project) branch master updated: JAMES-4043 Improve literal handling (#2311)
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
The following commit(s) were added to refs/heads/master by this push:
new 6f8bec686b JAMES-4043 Improve literal handling (#2311)
6f8bec686b is described below
commit 6f8bec686b9cc27d771b1eea6c9d553e13905cce
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Mon Jun 24 09:57:04 2024 +0200
JAMES-4043 Improve literal handling (#2311)
---
.../imapserver/netty/ImapRequestFrameDecoder.java | 61 ++++++++++++++---
.../james/imapserver/netty/IMAPServerTest.java | 79 +++++++++++++++++++++-
2 files changed, 130 insertions(+), 10 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 c5d9b6069a..14cb8aae92 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
@@ -26,6 +26,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
+import java.util.ArrayList;
import java.util.Deque;
import java.util.List;
import java.util.Map;
@@ -73,6 +74,7 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
public static final int UNAUTHENTICATE_LITERAL_MAX_SIZE = Optional.ofNullable(System.getProperty("james.imap.unauthenticated.literal.max.size"))
.map(Integer::parseInt)
.orElse(8192);
+ private static final String SHOULD_BUFFER = "should_buffer";
private final ImapDecoder decoder;
private final int inMemorySizeLimit;
@@ -80,6 +82,7 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
private final Deque<ChannelInboundHandlerAdapter> behaviourOverrides = new ConcurrentLinkedDeque<>();
private final int maxFrameLength;
private final AtomicBoolean framingEnabled = new AtomicBoolean(true);
+ private ArrayList<byte[]> pending = new ArrayList<>();
public ImapRequestFrameDecoder(ImapDecoder decoder, int inMemorySizeLimit, int literalSizeLimit, int maxFrameLength) {
this.decoder = decoder;
@@ -160,10 +163,15 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
enableFraming(ctx);
attachment.clear();
+ if (!pending.isEmpty()) {
+ pending.clear();
+ }
return Optional.of(message);
} catch (NettyImapRequestLineReader.NotEnoughDataException e) {
// this exception was thrown because we don't have enough data yet
requestMoreData(ctx, in, attachment, e.getNeededSize(), readerIndex);
+ } finally {
+ attachment.remove(SHOULD_BUFFER);
}
} else {
// The session was null so may be the case because the channel was already closed but there were still bytes in the buffer.
@@ -176,12 +184,25 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
}
private void requestMoreData(ChannelHandlerContext ctx, ByteBuf in, Map<String, Object> attachment, int neededData, int readerIndex) {
- // store the needed data size for later usage
- attachment.put(NEEDED_DATA, neededData);
+ Boolean shouldBuffer = Optional.ofNullable(attachment.get(SHOULD_BUFFER)).map(Boolean.class::cast).orElse(true);
+ if (shouldBuffer) {
+ // store the needed data size for later usage
+
+ int i = in.readerIndex();
+ in.readerIndex(readerIndex);
+
+ byte[] bytes = new byte[in.readableBytes()];
+ in.readBytes(bytes);
+ pending.add(bytes);
+ in.readerIndex(i);
+
+ attachment.put(NEEDED_DATA, neededData - bytes.length);
+ } else {
+ attachment.put(NEEDED_DATA, neededData);
+ }
// SwitchableDelimiterBasedFrameDecoder added further to JAMES-1436.
disableFraming(ctx);
- in.readerIndex(readerIndex);
}
private Pair<ImapRequestLineReader, Integer> obtainReader(ChannelHandlerContext ctx, ByteBuf in, Map<String, Object> attachment, int readerIndex) throws IOException {
@@ -195,7 +216,7 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
retry = true;
size = (Integer) rawSize;
// now see if the buffer hold enough data to process.
- if (size != NettyImapRequestLineReader.NotEnoughDataException.UNKNOWN_SIZE && size > in.readableBytes()) {
+ if (size != NettyImapRequestLineReader.NotEnoughDataException.UNKNOWN_SIZE) {
// check if we have a inMemorySize limit and if so if the
// expected size will fit into it
@@ -212,13 +233,32 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
return null;
} else {
- in.resetReaderIndex();
- return null;
+ int readableBytes = in.readableBytes();
+ byte[] bytes = new byte[readableBytes];
+ in.readBytes(bytes);
+ pending.add(bytes);
+
+ int totalSize = pending.stream().mapToInt(m -> m.length).sum();
+ if (totalSize >= size) {
+
+ ByteBuf byteBufs = Unpooled.wrappedBuffer(pending.toArray(byte[][]::new));
+ ImapSession session = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get();
+ attachment.put(SHOULD_BUFFER, false);
+ reader = new NettyImapRequestLineReader(ctx.channel(), byteBufs, retry, readerIndex, literalSizeLimit(session), maxFrameLength);
+ } else {
+ return null;
+ }
}
} else {
- ImapSession session = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get();
- reader = new NettyImapRequestLineReader(ctx.channel(), in, retry, readerIndex, literalSizeLimit(session), maxFrameLength);
+ if (pending.isEmpty()) {
+ ImapSession session = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get();
+ reader = new NettyImapRequestLineReader(ctx.channel(), in, retry, readerIndex, literalSizeLimit(session), maxFrameLength);
+ } else {
+ ByteBuf byteBufs = Unpooled.wrappedBuffer(pending.toArray(byte[][]::new));
+ ImapSession session = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get();
+ reader = new NettyImapRequestLineReader(ctx.channel(), byteBufs, retry, readerIndex, literalSizeLimit(session), maxFrameLength);
+ }
}
} else {
ImapSession session = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get();
@@ -238,7 +278,7 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
sink = Pair.of(Sinks.many().unicast().onBackpressureBuffer(), new AtomicInteger(0));
attachment.put(SINK, sink);
- FileChunkConsumer fileChunkConsumer = new FileChunkConsumer(size);
+ FileChunkConsumer fileChunkConsumer = new FileChunkConsumer(size + pending.stream().mapToInt(b -> b.length).sum());
Disposable subscribe = sink.getLeft().asFlux()
.publishOn(Schedulers.boundedElastic())
.subscribe(fileChunkConsumer,
@@ -268,6 +308,9 @@ public class ImapRequestFrameDecoder extends ByteToMessageDecoder implements Net
subscribe.dispose();
fileChunkConsumer.discard();
});
+
+ pending.forEach(bytes -> sink.getLeft().emitNext(bytes, FAIL_FAST));
+ pending.clear();
}
int readableBytes = in.readableBytes();
byte[] bytes = new byte[readableBytes];
diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
index 0218463cb6..50d075e30f 100644
--- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
+++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
@@ -110,6 +110,7 @@ import org.awaitility.Awaitility;
import org.eclipse.angus.mail.imap.IMAPFolder;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
@@ -1728,6 +1729,83 @@ class IMAPServerTest {
readStringUntil(clientConnection, s -> s.contains(("a2 BAD ")));
}
+ @Test
+ void passing2literalOnDifferentNetworkPackage() throws Exception {
+ MailboxSession mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+ memoryIntegrationResources.getMailboxManager()
+ .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+ clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a0 OK"));
+ clientConnection.write(ByteBuffer.wrap("a1 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a1 OK"));
+
+ String literal = "a".repeat(20);
+ clientConnection.write(ByteBuffer.wrap(("a2 SEARCH CHARSET UTF-8 TO {" + literal.length() + "+}\r\n").getBytes()));
+ clientConnection.write(ByteBuffer.wrap((literal + " TO {" + literal.length() + "+}\r\n").getBytes()));
+ clientConnection.write(ByteBuffer.wrap((literal + " ALL\r\n").getBytes()));
+
+ readStringUntil(clientConnection, s -> s.contains(("a2 OK ")));
+ }
+
+ @Test
+ void passing2literalOnSameNetworkPackage() throws Exception {
+ MailboxSession mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+ memoryIntegrationResources.getMailboxManager()
+ .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+ clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a0 OK"));
+ clientConnection.write(ByteBuffer.wrap("a1 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a1 OK"));
+
+ String literal = "a".repeat(16);
+ String s1 = "a2 SEARCH CHARSET UTF-8 TO {" + literal.length() + "+}\r\n" +
+ literal + " TO {" + literal.length() + "+}\r\n" + literal + " ALL\r\n";
+ clientConnection.write(ByteBuffer.wrap(s1.getBytes()));
+
+ readStringUntil(clientConnection, s -> s.contains(("a2 OK ")));
+ }
+
+ @Test
+ void passing2literalOnSameNetworkPackageWhenMoreThan16Chars() throws Exception {
+ MailboxSession mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+ memoryIntegrationResources.getMailboxManager()
+ .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+ clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a0 OK"));
+ clientConnection.write(ByteBuffer.wrap("a1 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a1 OK"));
+
+ String literal = "a".repeat(17);
+ String s1 = "a2 SEARCH CHARSET UTF-8 TO {" + literal.length() + "+}\r\n" +
+ literal + " TO {" + literal.length() + "+}\r\n" + literal + " ALL\r\n";
+ clientConnection.write(ByteBuffer.wrap(s1.getBytes()));
+
+ readStringUntil(clientConnection, s -> s.contains(("a2 OK ")));
+ }
+
+ @Disabled("JAMES-4043 Multiple literals and file literals are buggy")
+ @Test
+ void shouldAcceptSeveralFileLiteral() throws Exception {
+ MailboxSession mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+ memoryIntegrationResources.getMailboxManager()
+ .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+ clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a0 OK"));
+ clientConnection.write(ByteBuffer.wrap("a1 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8)));
+ readStringUntil(clientConnection, s -> s.contains("a1 OK"));
+
+ String litteral = "a".repeat(72 * 1024);
+ clientConnection.write(ByteBuffer.wrap(("a2 SEARCH CHARSET UTF-8 TO {" + litteral.length() + "+}\r\n").getBytes()));
+ clientConnection.write(ByteBuffer.wrap((litteral + " TO {2+}\r\n").getBytes()));
+ clientConnection.write(ByteBuffer.wrap(("aa ALL\r\n").getBytes()));
+
+ readStringUntil(clientConnection, s -> s.contains(("a2 OK ")));
+ }
+
@Test
void shouldRejectLongLineAfterLiteralWhenLogin() throws Exception {
MailboxSession mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
@@ -2627,7 +2705,6 @@ class IMAPServerTest {
readStringUntil(clientConnection, s -> s.contains("A1 OK [READ-WRITE] SELECT completed."));
clientConnection.write(ByteBuffer.wrap(("A2 UID FETCH 1:500 (BODY[])\r\n").getBytes(StandardCharsets.UTF_8)));
- Thread.sleep(1000);
assertThat(loaded.get()).isLessThan(500);
readStringUntil(clientConnection, s -> s.contains("A2 OK FETCH completed."));
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org