(geronimo-mail) branch main updated: GERONIMO-6892 - IMAP APPEND used to deadlock because the client never flushes the literal announcement
[email protected] Sat, 18 Jul 2026 09:24:47 +0000
| Newsgroups | gmane.comp.java.geronimo.cvs |
|---|---|
| Message-ID | <178436668706.1736390.3606941927729134479@gitbox3-he-fi.apache.org> |
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/geronimo-mail.git
The following commit(s) were added to refs/heads/main by this push:
new e2eada1 GERONIMO-6892 - IMAP APPEND used to deadlock because the client never flushes the literal announcement
e2eada1 is described below
commit e2eada121374ee53e6d608ecc39145ee918ef792
Author: Richard Zowalla <[email protected]>
AuthorDate: Sat Jul 18 11:24:16 2026 +0200
GERONIMO-6892 - IMAP APPEND used to deadlock because the client never flushes the literal announcement
---
.../mail/store/imap/connection/IMAPCommand.java | 4 +
.../mail/store/imap/connection/IMAPDateFormat.java | 8 +-
.../mail/store/imap/IMAPFolderAppendTest.java | 117 +++++++++++++++++++++
geronimo-mail_2.1_impl/tck.adoc | 9 +-
geronimo-mail_2.1_tck/pom.xml | 46 +-------
geronimo-mail_2.1_tck/src/tck/geronimo.jtx | 21 ++--
6 files changed, 146 insertions(+), 59 deletions(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPCommand.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPCommand.java
index 18416d8..00a0e4a 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPCommand.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPCommand.java
@@ -193,6 +193,10 @@ public class IMAPCommand {
// on to the end.
for (int i = 0; i < segments.size(); i++) {
outStream.write((byte [])segments.get(i));
+ // the segment (ending with a {n} literal announcement) must actually
+ // reach the server before we wait for its continuation response --
+ // without this flush both sides wait on each other forever.
+ outStream.flush();
// now wait for a response from the connection. We should be getting a
// continuation response back (and might have also received some asynchronous
// replies, which we'll leave in the queue for now. If we get some status back
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPDateFormat.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPDateFormat.java
index 75e766b..0bb9eba 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPDateFormat.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPDateFormat.java
@@ -41,11 +41,11 @@ public class IMAPDateFormat extends SimpleDateFormat {
}
public StringBuffer format(Date date, StringBuffer buffer, FieldPosition position) {
StringBuffer result = super.format(date, buffer, position);
- // The RFC 2060 requires that the day in the date be formatted with either 2 digits
- // or one digit. Our format specifies 2 digits, which pads with leading
- // zeros. We need to check for this and whack it if it's there
+ // RFC 3501 date-day-fixed is (SP DIGIT) / 2DIGIT: a single-digit day must
+ // be space-padded, not shortened. Our format pads with a leading zero,
+ // so replace that zero with the required space.
if (result.charAt(0) == '0') {
- result.deleteCharAt(0);
+ result.setCharAt(0, ' ');
}
return result;
}
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPFolderAppendTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPFolderAppendTest.java
new file mode 100644
index 0000000..2a50492
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPFolderAppendTest.java
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.geronimo.mail.store.imap;
+
+import java.util.Calendar;
+import java.util.Date;
+import java.util.Properties;
+import java.util.TimeZone;
+
+import jakarta.mail.Folder;
+import jakarta.mail.Message;
+import jakarta.mail.Session;
+import jakarta.mail.Store;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
+
+import org.apache.geronimo.mail.testserver.AbstractProtocolTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Regression test for IMAP APPEND against Apache James.
+ *
+ * IMAPCommand.writeTo used to write the command segment announcing a literal
+ * ("APPEND ... {n}") into the buffered output stream and then block waiting
+ * for the server's continuation response WITHOUT flushing the stream: the
+ * announcement never reached the server, so client and server waited on each
+ * other forever. The timeout on these tests guards against a regression of
+ * that deadlock. A second defect fixed alongside: the INTERNALDATE sent with
+ * APPEND shortened single-digit days ("8-Dec-...") where RFC 3501 requires
+ * space padding (" 8-Dec-..."), which strict servers reject with BAD.
+ */
+public class IMAPFolderAppendTest extends AbstractProtocolTest {
+
+ private Store connect() throws Exception {
+ final Properties props = new Properties();
+ props.setProperty("mail.imap.port", String.valueOf(imapConf.getListenerPort()));
+ props.setProperty("mail.debug", "true");
+ final Session session = Session.getInstance(props);
+ final Store store = session.getStore("imap");
+ store.connect("127.0.0.1", "serveruser", "serverpass");
+ return store;
+ }
+
+ private MimeMessage message(final String subject, final Date sentDate) throws Exception {
+ final MimeMessage msg = new MimeMessage(Session.getInstance(new Properties()));
+ msg.setFrom(new InternetAddress("serveruser@localhost"));
+ msg.setRecipient(Message.RecipientType.TO, new InternetAddress("serveruser@localhost"));
+ msg.setSubject(subject);
+ msg.setSentDate(sentDate);
+ msg.setText("append test body");
+ return msg;
+ }
+
+ @Test
+ @Timeout(60)
+ public void testAppendMessages() throws Exception {
+ start();
+
+ final Store store = connect();
+ try {
+ final Folder inbox = store.getFolder("INBOX");
+ inbox.open(Folder.READ_WRITE);
+ assertEquals(0, inbox.getMessageCount());
+
+ inbox.appendMessages(new Message[] { message("appended", new Date()) });
+
+ assertEquals(1, inbox.getMessageCount());
+ assertEquals("appended", inbox.getMessage(1).getSubject());
+ inbox.close(false);
+ } finally {
+ store.close();
+ }
+ }
+
+ @Test
+ @Timeout(60)
+ public void testAppendMessageWithSingleDigitDay() throws Exception {
+ start();
+
+ // a sent date on a single-digit day exercises the RFC 3501
+ // space-padded INTERNALDATE form
+ final Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+ cal.set(2025, Calendar.DECEMBER, 8, 12, 0, 0);
+
+ final Store store = connect();
+ try {
+ final Folder inbox = store.getFolder("INBOX");
+ inbox.open(Folder.READ_WRITE);
+
+ inbox.appendMessages(new Message[] { message("single digit day", cal.getTime()) });
+
+ assertEquals(1, inbox.getMessageCount());
+ assertEquals("single digit day", inbox.getMessage(1).getSubject());
+ inbox.close(false);
+ } finally {
+ store.close();
+ }
+ }
+
+}
diff --git a/geronimo-mail_2.1_impl/tck.adoc b/geronimo-mail_2.1_impl/tck.adoc
index 974a748..183ad7d 100644
--- a/geronimo-mail_2.1_impl/tck.adoc
+++ b/geronimo-mail_2.1_impl/tck.adoc
@@ -7,11 +7,10 @@ JavaTest harness. Pass `-Dtck.failIgnore=false` to make TCK failures fail the
build. The manual setup below remains as reference for interactive/GUI runs and
for debugging individual tests.
-KNOWN ISSUE: populating the mailbox through the geronimo IMAP client deadlocks
-in `IMAPConnection.appendMessage` (the client blocks waiting for the server's
-literal continuation that never arrives - reproducible against Apache James 3.9).
-The tck module therefore runs `fpopulate` with the Angus RI client (fixture
-setup only). The same defect likely affects the TCK's own APPEND-related tests.
+RESOLVED (2026-07-18): IMAP APPEND used to deadlock because the client never
+flushed the literal announcement before waiting for the server's continuation
+(fixed in IMAPCommand.writeTo, along with the RFC 3501 space-padded
+INTERNALDATE day). `fpopulate` runs through the geronimo client again.
# Docker CTS Mailserver setup
diff --git a/geronimo-mail_2.1_tck/pom.xml b/geronimo-mail_2.1_tck/pom.xml
index 2e796d0..4ebabe8 100644
--- a/geronimo-mail_2.1_tck/pom.xml
+++ b/geronimo-mail_2.1_tck/pom.xml
@@ -226,43 +226,6 @@
<outputDirectory>${project.build.directory}/jarpath</outputDirectory>
</configuration>
</execution>
- <!-- fixture client used only to populate the test1 mailbox:
- population through the geronimo IMAP client currently hangs
- in APPEND (continuation deadlock, known issue - see tck.adoc),
- and mailbox population is test setup, not the system under test -->
- <execution>
- <id>copy-fixture</id>
- <phase>generate-test-resources</phase>
- <goals>
- <goal>copy</goal>
- </goals>
- <configuration>
- <artifactItems>
- <artifactItem>
- <groupId>jakarta.mail</groupId>
- <artifactId>jakarta.mail-api</artifactId>
- <version>2.1.3</version>
- </artifactItem>
- <artifactItem>
- <groupId>jakarta.activation</groupId>
- <artifactId>jakarta.activation-api</artifactId>
- <version>2.1.3</version>
- </artifactItem>
- <artifactItem>
- <groupId>org.eclipse.angus</groupId>
- <artifactId>angus-mail</artifactId>
- <version>2.0.3</version>
- </artifactItem>
- <artifactItem>
- <groupId>org.eclipse.angus</groupId>
- <artifactId>angus-activation</artifactId>
- <version>2.0.2</version>
- </artifactItem>
- </artifactItems>
- <outputDirectory>${project.build.directory}/fixture</outputDirectory>
- <stripVersion>true</stripVersion>
- </configuration>
- </execution>
</executions>
</plugin>
@@ -340,13 +303,14 @@
</waitfor>
<fail message="embedded James did not open ports ${tck.smtp.port}/${tck.imap.port} in time" if="tck.james.timeout"/>
- <!-- fixture setup: fill mailbox 'test1' with the TCK's own data -->
+ <!-- fixture setup: fill mailbox 'test1' with the TCK's own data,
+ using the geronimo client itself (the APPEND deadlock that
+ once forced the Angus RI here is fixed) -->
<java classname="fpopulate" fork="true" dir="${tck.home}/tests/mailboxes" failonerror="true" timeout="120000">
<classpath>
<pathelement location="${tck.home}/tests/mailboxes"/>
- <fileset dir="${project.build.directory}/fixture">
- <include name="*.jar"/>
- </fileset>
+ <pathelement location="${project.build.directory}/jarpath/geronimo-mail_2.1_mail.jar"/>
+ <pathelement location="${project.build.directory}/jarpath/geronimo-activation_2.0_spec.jar"/>
</classpath>
<arg value="-s"/>
<arg value="test1"/>
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index b9c46cd..472e33c 100644
--- a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
+++ b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
@@ -22,15 +22,6 @@
# Baseline recorded 2026-07-18 against Jakarta Mail TCK 2.1.1 / James 3.9.0.
#
-# IMAP APPEND continuation deadlock: the client blocks forever waiting for the
-# server's literal continuation (see tck.adoc "KNOWN ISSUE"). Every test below
-# invokes Folder.appendMessages through the geronimo IMAP client.
-javasoft/sqe/tests/jakarta/mail/event/FolderEvent/testlist.html#addMsgCntList_Test
-javasoft/sqe/tests/jakarta/mail/exception/testlist.html#illegalWriteException_Test
-javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#appendMessages_Test
-javasoft/sqe/tests/jakarta/mail/UIDFolder/testlist.html#getUID_Test
-javasoft/sqe/tests/jakarta/mail/UIDFolder/testlist.html#getUIDNext_Test
-
# API signature verification: needs a dedicated sigtest setup (signature
# records, sigtest.jar invocation) and is out of scope for the functional
# gate; currently reports 18 signature mismatches in the spec classes that
@@ -38,6 +29,18 @@ javasoft/sqe/tests/jakarta/mail/UIDFolder/testlist.html#getUIDNext_Test
SignatureTest.html
+# Folder.close(false) must not expunge: the client issues CLOSE, which per
+# RFC 3501 always expunges \Deleted messages; not expunging requires UNSELECT
+# (or dropping the selection another way). Surfaced once the APPEND deadlock
+# was fixed (appendMessages_Test unit 3 appends \Deleted-flagged messages and
+# expects them to survive close(false)).
+javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#appendMessages_Test
+
+# MessageCountListener events are queued to the wrong listener list in
+# jakarta.mail.Folder (notifyMessageAdded/RemovedListeners use
+# messageChangedListeners), so count listeners never fire.
+javasoft/sqe/tests/jakarta/mail/event/FolderEvent/testlist.html#addMsgCntList_Test
+
# Remaining baseline failures (284 passed / 31 failed after the
# IMAPFolder.renameTo fix removed a ~90-test cascade). Distinct defects:
# folder create/delete (topdog), permanent flags, list/listSubscribed,