gkm2164 opened a new pull request, #3144:
URL: https://github.com/apache/james-project/pull/3144
## Problem
When a client pipelines two commands using LITERAL+ (RFC 7888) - e.g. two
`APPEND`s back to back without waiting for a response in between, both
delivered to the server in a single TCP read - the **second command's
bytes are silently dropped**. No error, no timeout signal to the client,
no server-side log trace: the tag is simply never answered, and the
message is never persisted.
Minimal repro (raw IMAP, after `LOGIN`):
```
a1 APPEND INBOX {46+}
From: [email protected]
Subject: msg1
body1
a2 APPEND INBOX {46+}
From: [email protected]
Subject: msg2
body2
```
sent as one write. `a1 OK ... APPEND completed.` arrives; `a2` never does,
and `INBOX` only gains one new message, not two.
## Root cause
`ImapRequestFrameDecoder.obtainReader()` drains the *entire* readable
buffer into `pending` once it knows a literal's exact byte count, not just
the bytes the literal actually needs:
```java
int readableBytes = in.readableBytes(); // whole remaining buffer, incl. the
next command
byte[] bytes = new byte[readableBytes];
in.readBytes(bytes);
pending.add(bytes);
```
Once the literal parses successfully, `parseImapMessage()` unconditionally
clears `pending`:
```java
if (!pending.isEmpty()) {
pending.clear(); // discards whatever of the next command was over-read
}
```
`pending` is a plain `List<byte[]>` snapshot with no notion of how much
the `ImapRequestLineReader` actually consumed from it, so any excess -
including a fully-formed next command sitting right behind the literal -
is gone.
This is a regression from 6f8bec68 (JAMES-4043, "Improve literal
handling"). Before that change, this path only ran when the literal
genuinely wasn't fully available yet (`size > in.readableBytes()`); when
enough data was already present in one read, `obtainReader()` built the
`ImapRequestLineReader` directly over Netty's own cumulation buffer `in`,
so the literal parser's own consumption naturally left the rest of `in`
(the next pipelined command) in place for `ByteToMessageDecoder`'s decode
loop to pick up on its next iteration. JAMES-4043 routed every path
through the always-buffer-into-`pending` mechanism - correct for the
slow/multi-read arrival it targeted, but it breaks the single-read case
where more than one command's worth of bytes is already available.
The same over-read pattern exists in `uploadToAFile()` (the
`inMemorySizeLimit`-exceeding path): it reads the whole buffer before
`FileChunkConsumer` caps what actually reaches the temp file, so a
pipelined command behind a large literal is read out of `in` and lost the
same way. Fixed there too.
## Fix
Only consume as many bytes as still needed to reach the literal's declared
size in both places, leaving any excess unread in `in` (or unconsumed from
the socket) for the existing decode loop to pick up as the next frame -
restoring the pre-JAMES-4043 behavior for the "already enough data"
case without touching the JAMES-4043 improvement for genuinely
slow/multi-read
arrivals.
## Testing
- Added
`IMAPServerTest.PipelinedLiteralAppend#secondPipelinedLiteralAppendShouldNotBeSilentlyDropped`:
two LITERAL+ `APPEND`s pipelined in one write; asserts both get a tagged
`OK ... APPEND completed` response (time-bounded, since without the fix
the second response never arrives at all).
- Manually verified against a live DynamoDB-backed James 3.9.0 deployment
where `imaptest` (Dovecot) first surfaced the drop under concurrent
load with LITERAL+ `APPEND`s carrying custom keyword flags; isolated to
this exact two-commands-in-one-read pipelining pattern via a minimal
raw-socket repro, traced to this method with instrumented local builds,
and confirmed the fix resolves both the missing response and the missing
mailbox data (`EXISTS` count) before opening this PR.
Marking as draft since this is a first-time contribution - happy to adjust
approach, add more test coverage (e.g. the `uploadToAFile` path
specifically), or split into two commits if that's preferred.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
