prosgarz35 commented on PR #3198:
URL: https://github.com/apache/james-project/pull/3198#issuecomment-5807480065
### Final Polish & Comprehensive Integration Test Verification
We have addressed the remaining lifecycle concerns and added targeted
end-to-end integration tests covering all requested edge cases:
1. **Immediate `idleReadySink` Completion on Teardown**:
- `idleReadySink.tryEmitEmpty()` is now invoked directly within
`cleanupIdle(...)`. Disconnections, heartbeat socket errors, or delivery write
failures promptly complete the reactive sink and release all retained listener
and
session references.
2. **Strict RFC 2177 Continuation Compliance**:
- Removed artificial custom command matching in `IdleProcessor`. All
continuation data other than `"DONE"` strictly triggers RFC-compliant tagged
`BAD INVALID_CONTINUATION`, cleanly exits IDLE via `cleanupIdle(...)`, and
restores the session for subsequent standard command processing.
3. **New Integration Tests Added & Verified (`IMAPServerIdleTest`)**:
- `invalidContinuationShouldEndIdleAndAllowSubsequentCommands`:
Verifies that receiving an invalid continuation emits tagged `BAD`, cleanly
pops the line handler, and allows subsequent commands (e.g. `NOOP`) to succeed
normally.
- `midIdleLogoutShouldRejectContinuationAndAllowSubsequentLogout`:
Verifies that sending `LOGOUT` as a continuation payload rejects IDLE with
tagged `BAD`, after which a subsequent standard tagged `LOGOUT` command
completes
cleanly with `* BYE`.
- `disconnectDuringIdleShouldCleanlyDecrementConnections`: Verifies
that an abrupt TCP client disconnect during active IDLE cleanly decrements the
connection metric back to 0 without leaking server sessions.
#### Local Test Run Results
- `IMAPServerIdleTest`: **61 / 61 passed (0 failures, 0 errors)**
- `IMAPServerIdleSSLTest`: **55 / 55 passed (0 failures, 0 errors)**
- `IMAPServerIdleSSLCompressTest`: **1 / 1 passed (0 failures, 0
errors)**
- Checkstyle: **0 violations across all modified modules**.
### Detailed Integration Test Suite Coverage for IDLE Edge Cases
To provide full confidence in production readiness, we added explicit
integration test cases to
`server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerIdleTest.java`:
#### 1. Invalid Continuation Recovery Test
```java
@Test
void invalidContinuationShouldEndIdleAndAllowSubsequentCommands() throws
Exception {
clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s
%s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
readBytes(clientConnection);
clientConnection.write(ByteBuffer.wrap(("a2 SELECT
INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
readStringUntil(clientConnection, s -> s.contains("a2 OK
[READ-WRITE] SELECT completed."));
// Issue IDLE followed by an invalid continuation command
clientConnection.write(ByteBuffer.wrap(("a3
IDLE\r\nINVALID\r\n").getBytes(StandardCharsets.UTF_8)));
// Expect tagged BAD response for IDLE
Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
assertThat(readStringUntil(clientConnection, s -> s.contains("a3
BAD IDLE failed.")))
.isNotNull());
// Subsequent command must succeed normally, proving line handler
was cleanly popped
clientConnection.write(ByteBuffer.wrap(("a4
NOOP\r\n").getBytes(StandardCharsets.UTF_8)));
Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
assertThat(readStringUntil(clientConnection, s -> s.contains("a4
OK NOOP completed.")))
.isNotNull());
}
• Verified Behavior: Confirms that non-DONE continuations do not get
trapped in an infinite continuation loop and that popLineHandler() correctly
reinstates the standard IMAP command parser.
#### Continuation LOGOUT Rejection & Subsequent Command Execution
@Test
void midIdleLogoutShouldRejectContinuationAndAllowSubsequentLogout()
throws Exception {
clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s
%s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
readBytes(clientConnection);
clientConnection.write(ByteBuffer.wrap(("a2 SELECT
INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
readStringUntil(clientConnection, s -> s.contains("a2 OK
[READ-WRITE] SELECT completed."));
clientConnection.write(ByteBuffer.wrap(("a3
IDLE\r\n").getBytes(StandardCharsets.UTF_8)));
readStringUntil(clientConnection, s -> s.contains("+ Idling"));
// Sending unexpected continuation during IDLE
clientConnection.write(ByteBuffer.wrap(("LOGOUT\r\n").getBytes(StandardCharsets.UTF_8)));
// Server should reject IDLE with BAD
Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
assertThat(readStringUntil(clientConnection, s -> s.contains("a3
BAD IDLE failed. Continuation for IMAP IDLE was not understood. Expected
'DONE', got 'LOGOUT'.")))
.isNotNull());
// Subsequent tagged LOGOUT command must succeed normally
clientConnection.write(ByteBuffer.wrap(("a4
LOGOUT\r\n").getBytes(StandardCharsets.UTF_8)));
Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
assertThat(readStringUntil(clientConnection, s -> s.contains("a4
OK LOGOUT completed.")))
.isNotNull());
}
• Verified Behavior: Verifies that clients attempting to disconnect
mid-IDLE receive RFC-mandated BAD rejection while cleanly exiting IDLE,
allowing standard teardown via the standard IMAP processor pipeline.
#### Abrupt Client Disconnection Cleanup Test
@Test
void disconnectDuringIdleShouldCleanlyDecrementConnections() throws
Exception {
clientConnection.write(ByteBuffer.wrap(String.format("a0 LOGIN %s
%s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
readBytes(clientConnection);
clientConnection.write(ByteBuffer.wrap(("a2 SELECT
INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
readStringUntil(clientConnection, s -> s.contains("a2 OK
[READ-WRITE] SELECT completed."));
clientConnection.write(ByteBuffer.wrap(("a3
IDLE\r\n").getBytes(StandardCharsets.UTF_8)));
readStringUntil(clientConnection, s -> s.contains("+ Idling"));
// Abruptly sever connection
clientConnection.close();
// Verify connection metric decrements back to 0
Awaitility.await().atMost(Duration.ofSeconds(5)).untilAsserted(() ->
assertThat(metricFactory.countFor("imapConnections")).isZero());
}
• Verified Behavior: Confirms that abruptly severed client TCP connections
while idling cleanly release sessions and metrics without ghost references.
All tests passed with 100% success (Total: 117 tests across all IMAP idle
test suites).
--
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]
