You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by rz...@apache.org on 2026/07/18 19:12:26 UTC
(geronimo-mail) branch main updated (38a455a -> beafafb)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/geronimo-mail.git
from 38a455a GERONIMO-6901 - Failed Folder.open() leaks the pooled connection, breaks Store.close() and hides FolderNotFoundException
new 30c7fc6 GERONIMO-6901 - harden the failed-open unwind: deselect a successfully selected mailbox before pooling the connection
new b78b1e6 GERONIMO-6902 - Folder.expunge() always returns an empty array - EXPUNGE responses extracted under the wrong keyword
new 3a4e881 GERONIMO-6903 - IMAP response parser drops the byte following a bare CR, corrupting the stream
new 2392bdf GERONIMO-6904 - A garbled IMAP response leaves the connection poolable and poisons later commands
new beafafb GERONIMO-6905 - IMAP response codes in brackets (PERMANENTFLAGS, UIDVALIDITY, UIDNEXT, UNSEEN) are never parsed
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../geronimo/mail/store/imap/IMAPFolder.java | 17 +++-
.../mail/store/imap/connection/IMAPConnection.java | 21 +++-
.../store/imap/connection/IMAPMailboxStatus.java | 4 +-
.../store/imap/connection/IMAPResponseStream.java | 27 ++++--
.../mail/store/imap/IMAPExpungeRegressionTest.java | 107 +++++++++++++++++++++
.../imap/IMAPPermanentFlagsRegressionTest.java | 94 ++++++++++++++++++
.../IMAPConnectionBrokenResponseTest.java | 86 +++++++++++++++++
.../imap/connection/IMAPOkResponseParsingTest.java | 100 +++++++++++++++++++
.../imap/connection/IMAPResponseStreamTest.java | 94 ++++++++++++++++++
geronimo-mail_2.1_tck/src/tck/geronimo.jtx | 2 -
10 files changed, 537 insertions(+), 15 deletions(-)
create mode 100644 geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPExpungeRegressionTest.java
create mode 100644 geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPPermanentFlagsRegressionTest.java
create mode 100644 geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnectionBrokenResponseTest.java
create mode 100644 geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPOkResponseParsingTest.java
create mode 100644 geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStreamTest.java
(geronimo-mail) 05/05: GERONIMO-6905 - IMAP response codes in brackets (PERMANENTFLAGS, UIDVALIDITY, UIDNEXT, UNSEEN) are never parsed
Posted by rz...@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
commit beafafb7720ceb78cafbe70aaac36bab15da1a4e
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Jul 18 21:10:28 2026 +0200
GERONIMO-6905 - IMAP response codes in brackets (PERMANENTFLAGS, UIDVALIDITY, UIDNEXT, UNSEEN) are never parsed
---
.../store/imap/connection/IMAPMailboxStatus.java | 4 +-
.../store/imap/connection/IMAPResponseStream.java | 17 ++--
.../imap/IMAPPermanentFlagsRegressionTest.java | 94 +++++++++++++++++++
.../imap/connection/IMAPOkResponseParsingTest.java | 100 +++++++++++++++++++++
geronimo-mail_2.1_tck/src/tck/geronimo.jtx | 1 -
5 files changed, 207 insertions(+), 9 deletions(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPMailboxStatus.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPMailboxStatus.java
index 3d6646f..b12e7b4 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPMailboxStatus.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPMailboxStatus.java
@@ -163,8 +163,8 @@ public class IMAPMailboxStatus {
}
// untagged unseen response
else if (source.isKeyword("UNSEEN")) {
- List arguments = source.getStatus();
- uidValidity = ((Token)arguments.get(0)).getInteger();
+ List arguments = source.getStatus();
+ unseenMessages = ((Token)arguments.get(0)).getInteger();
}
}
}
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
index 221031c..f6f0a44 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
@@ -239,7 +239,11 @@ public class IMAPResponseStream {
* @return An IMAPResponse instance for this message.
*/
private IMAPResponse parseUntaggedOkResponse(byte [] data, IMAPResponseTokenizer tokenizer) throws MessagingException {
- Token token = tokenizer.peek();
+ // NB: '[' and ']' are only delimiters in the expanded delimiter set, so all
+ // tokens of the response code section must be read in that mode. With the
+ // default atom delimiters "[PERMANENTFLAGS" would come back as a single
+ // ATOM token and the response code would never be recognized.
+ Token token = tokenizer.peek(false, true);
// we might have an optional value here
if (token.getType() != '[') {
// this has no tagging item, so there's nothing to be processed
@@ -247,8 +251,8 @@ public class IMAPResponseStream {
return new IMAPOkResponse("OK", null, tokenizer.getRemainder(), data);
}
// skip over the "[" token
- tokenizer.next();
- token = tokenizer.next();
+ tokenizer.next(false, true);
+ token = tokenizer.next(false, true);
String keyword = token.getValue();
// Permanent flags gets special handling
@@ -259,10 +263,11 @@ public class IMAPResponseStream {
ArrayList arguments = new ArrayList();
// strip off all of the argument tokens until the "]" list terminator.
- token = tokenizer.next();
- while (token.getType() != ']') {
+ // guard against EOF in case the terminator is missing from the response.
+ token = tokenizer.next(false, true);
+ while (token.getType() != ']' && token.getType() != Token.EOF) {
arguments.add(token);
- token = tokenizer.next();
+ token = tokenizer.next(false, true);
}
// this has a tagged keyword and arguments that will be processed later.
return new IMAPOkResponse(keyword, arguments, tokenizer.getRemainder(), data);
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPPermanentFlagsRegressionTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPPermanentFlagsRegressionTest.java
new file mode 100644
index 0000000..3063d91
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPPermanentFlagsRegressionTest.java
@@ -0,0 +1,94 @@
+/**
+ * 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.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.util.Properties;
+
+import jakarta.mail.Flags;
+import jakarta.mail.Folder;
+import jakarta.mail.Session;
+import jakarta.mail.Store;
+
+import org.apache.geronimo.mail.testserver.AbstractProtocolTest;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression test for the respcode-bracket-parsing defect surfaced by the
+ * Jakarta Mail TCK (Folder#getPermanentFlags_Test): the '* OK
+ * [PERMANENTFLAGS ...]', '* OK [UIDVALIDITY ...]' etc. response codes sent
+ * during SELECT collapsed into generic OK responses because '[' was not a
+ * delimiter in the tokenizer's default atom mode, so getPermanentFlags()
+ * always returned null and the open-time UID metadata was lost.
+ */
+public class IMAPPermanentFlagsRegressionTest extends AbstractProtocolTest {
+
+ protected 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;
+ }
+
+ protected byte[] readMessageResource(final String name) throws Exception {
+ final ByteArrayOutputStream bout = new ByteArrayOutputStream();
+ try (InputStream in = IMAPPermanentFlagsRegressionTest.class.getResourceAsStream(name)) {
+ final byte[] buf = new byte[4096];
+ int n;
+ while ((n = in.read(buf)) != -1) {
+ bout.write(buf, 0, n);
+ }
+ }
+ return bout.toByteArray();
+ }
+
+ @Test
+ public void testPermanentFlagsAndUidValidityAfterOpen() throws Exception {
+ start();
+ server.createUserMailbox("test1");
+ server.appendToUserMailbox("test1", readMessageResource("/messages/simple.msg"));
+
+ final Store store = connect();
+ try {
+ final IMAPFolder folder = (IMAPFolder) store.getDefaultFolder().getFolder("test1");
+ folder.open(Folder.READ_WRITE);
+ try {
+ final Flags permanentFlags = folder.getPermanentFlags();
+ assertNotNull(permanentFlags,
+ "getPermanentFlags() must reflect the PERMANENTFLAGS response code from SELECT");
+ assertTrue(permanentFlags.contains(Flags.Flag.SEEN),
+ "the permanent flags advertised by James must include \\Seen");
+ assertTrue(permanentFlags.contains(Flags.Flag.DELETED),
+ "the permanent flags advertised by James must include \\Deleted");
+
+ assertTrue(folder.getUIDValidity() > 0,
+ "getUIDValidity() must be populated from the UIDVALIDITY response code");
+ } finally {
+ folder.close(false);
+ }
+ } finally {
+ store.close();
+ }
+ }
+}
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPOkResponseParsingTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPOkResponseParsingTest.java
new file mode 100644
index 0000000..4ea0167
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPOkResponseParsingTest.java
@@ -0,0 +1,100 @@
+/**
+ * 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.connection;
+
+import java.io.ByteArrayInputStream;
+
+import jakarta.mail.Flags;
+
+import org.apache.geronimo.mail.store.imap.connection.IMAPResponseTokenizer.Token;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression tests for the respcode-bracket-parsing defect: '[' is not a
+ * delimiter in the tokenizer's default atom mode, so "[PERMANENTFLAGS"
+ * arrived as a single ATOM token and every '* OK [keyword args]' response
+ * code collapsed into a generic OK response. getPermanentFlags() therefore
+ * always returned null and the open-time UIDVALIDITY/UIDNEXT/UNSEEN metadata
+ * was silently lost.
+ */
+public class IMAPOkResponseParsingTest {
+
+ private IMAPResponse parse(final String line) throws Exception {
+ final IMAPResponseStream stream =
+ new IMAPResponseStream(new ByteArrayInputStream(line.getBytes("ISO8859-1")));
+ return stream.readResponse();
+ }
+
+ @Test
+ public void testPlainOkResponseHasOkKeyword() throws Exception {
+ final IMAPOkResponse response = (IMAPOkResponse) parse("* OK ready to serve\r\n");
+ assertTrue(response.isKeyword("OK"));
+ assertEquals("ready to serve", response.getMessage().trim());
+ }
+
+ @Test
+ public void testUidValidityResponseCodeIsRecognized() throws Exception {
+ final IMAPOkResponse response = (IMAPOkResponse) parse("* OK [UIDVALIDITY 137045278] UIDs valid\r\n");
+ assertTrue(response.isKeyword("UIDVALIDITY"),
+ "the response code inside the brackets must become the response keyword");
+ assertEquals(137045278L, ((Token) response.getStatus().get(0)).getLong());
+ }
+
+ @Test
+ public void testUidNextResponseCodeIsRecognized() throws Exception {
+ final IMAPOkResponse response = (IMAPOkResponse) parse("* OK [UIDNEXT 4] Predicted next UID\r\n");
+ assertTrue(response.isKeyword("UIDNEXT"));
+ assertEquals(4L, ((Token) response.getStatus().get(0)).getLong());
+ }
+
+ @Test
+ public void testUnseenResponseCodeIsRecognized() throws Exception {
+ final IMAPOkResponse response = (IMAPOkResponse) parse("* OK [UNSEEN 3] Message 3 is first unseen\r\n");
+ assertTrue(response.isKeyword("UNSEEN"));
+ assertEquals(3, ((Token) response.getStatus().get(0)).getInteger());
+ }
+
+ @Test
+ public void testPermanentFlagsResponseCodeIsRecognized() throws Exception {
+ final IMAPPermanentFlagsResponse response = (IMAPPermanentFlagsResponse)
+ parse("* OK [PERMANENTFLAGS (\\Answered \\Deleted \\Seen \\*)] Limited\r\n");
+ assertTrue(response.isKeyword("PERMANENTFLAGS"));
+ assertTrue(response.flags.contains(Flags.Flag.SEEN));
+ assertTrue(response.flags.contains(Flags.Flag.ANSWERED));
+ assertTrue(response.flags.contains(Flags.Flag.DELETED));
+ assertTrue(response.flags.contains(Flags.Flag.USER),
+ "the \\* wildcard must map to the USER flag");
+ }
+
+ @Test
+ public void testMailboxStatusMergesOkResponseCodes() throws Exception {
+ final IMAPMailboxStatus status = new IMAPMailboxStatus();
+ status.mergeStatus((IMAPOkResponse) parse("* OK [UIDVALIDITY 137045278] UIDs valid\r\n"));
+ status.mergeStatus((IMAPOkResponse) parse("* OK [UIDNEXT 4] Predicted next UID\r\n"));
+ status.mergeStatus((IMAPOkResponse) parse("* OK [UNSEEN 3] first unseen\r\n"));
+
+ assertEquals(137045278L, status.uidValidity);
+ assertEquals(4L, status.uidNext);
+ // regression: the UNSEEN branch used to assign uidValidity instead
+ assertEquals(3, status.unseenMessages);
+ assertEquals(137045278L, status.uidValidity,
+ "merging UNSEEN must not clobber the uidValidity value");
+ }
+}
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index 945b564..1788637 100644
--- a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
+++ b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
@@ -33,7 +33,6 @@ SignatureTest.html
# IMAPFolder.renameTo fix removed a ~90-test cascade). Distinct defects:
# folder create/delete (topdog), permanent flags, list/listSubscribed,
# several MIME encoding/decoding and unicode behaviours.
-javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#getPermanentFlags_Test
javasoft/sqe/tests/jakarta/mail/internet/InternetAddress/testlist.html#unicode_Test
javasoft/sqe/tests/jakarta/mail/internet/MimeBodyPart/testlist.html#attachFile_saveFile_Test
javasoft/sqe/tests/jakarta/mail/internet/MimeBodyPart/testlist.html#isMimeType_Test
(geronimo-mail) 01/05: GERONIMO-6901 - harden the failed-open unwind: deselect a successfully selected mailbox before pooling the connection
Posted by rz...@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
commit 30c7fc621616c317ee51678fe68a6c808b674765
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Jul 18 20:54:38 2026 +0200
GERONIMO-6901 - harden the failed-open unwind: deselect a successfully selected mailbox before pooling the connection
Follow-up from review: on the ReadOnlyFolderException sub-path the
SELECT succeeded, so the unwind returned a pooled connection with the
mailbox still selected. Track selection state and issue a best-effort
CLOSE before releasing. Also guard removeResponseHandler so an
unexpected failure cannot skip releaseFolderConnection.
---
.../org/apache/geronimo/mail/store/imap/IMAPFolder.java | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
index 52dfcbc..e36a292 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
@@ -651,6 +651,9 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
// record our open mode
this.mode = mode;
+ // tracks whether SELECT/EXAMINE succeeded, so the failure unwind knows
+ // if the connection still has a mailbox attached
+ boolean mailboxSelected = false;
try {
IMAPMailboxStatus status;
@@ -675,6 +678,7 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
}
throw e;
}
+ mailboxSelected = true;
// not available in the requested mode?
if (status.mode != mode) {
@@ -729,8 +733,19 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
} catch (MessagingException e) {
// ignore...we're already on a failure path here.
}
- conn.removeResponseHandler(this);
+ if (mailboxSelected) {
+ try {
+ // the SELECT/EXAMINE itself succeeded (e.g. the
+ // ReadOnlyFolderException path), so the mailbox is still
+ // selected; deselect so the connection goes back to the
+ // pool without a live mailbox attached.
+ conn.closeMailbox();
+ } catch (MessagingException e) {
+ // ignore...we're already on a failure path here.
+ }
+ }
try {
+ conn.removeResponseHandler(this);
((IMAPStore)store).releaseFolderConnection(this, conn);
} catch (MessagingException e) {
// ignore...we're already on a failure path here.
(geronimo-mail) 03/05: GERONIMO-6903 - IMAP response parser drops the byte following a bare CR, corrupting the stream
Posted by rz...@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
commit 3a4e881dde8a8f405c8497052b50482d4c73e011
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Jul 18 21:10:28 2026 +0200
GERONIMO-6903 - IMAP response parser drops the byte following a bare CR, corrupting the stream
---
.../store/imap/connection/IMAPResponseStream.java | 10 ++-
.../imap/connection/IMAPResponseStreamTest.java | 94 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
index cc5ac04..221031c 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStream.java
@@ -295,8 +295,8 @@ public class IMAPResponseStream {
* @exception IOException
*/
public void readBuffer() throws MessagingException {
+ int ch = nextByte();
while (true) {
- int ch = nextByte();
// potential end of line? Check the next character, and if it is an end of line,
// we need to do literal processing.
if (ch == '\r') {
@@ -307,9 +307,17 @@ public class IMAPResponseStream {
checkLiteral();
return;
}
+ // a bare CR that is not part of a CRLF sequence. The CR is part of
+ // the response data, so write it out, then reprocess the look-ahead
+ // byte on the next loop iteration (it might itself be a CR starting
+ // a real CRLF line terminator, so it must not be consumed here).
+ out.write(ch);
+ ch = next;
+ continue;
}
// write this to the buffer.
out.write(ch);
+ ch = nextByte();
}
}
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStreamTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStreamTest.java
new file mode 100644
index 0000000..a1ae347
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPResponseStreamTest.java
@@ -0,0 +1,94 @@
+/**
+ * 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.connection;
+
+import java.io.ByteArrayInputStream;
+import java.io.UnsupportedEncodingException;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Unit tests for the low level response line reader. Mainly a regression
+ * test for the resp-cr-drop defect: a bare CR followed by a non-LF byte used
+ * to silently drop the look-ahead byte, corrupting the stream (and a CR CR LF
+ * sequence could prevent line termination entirely).
+ */
+public class IMAPResponseStreamTest {
+
+ private IMAPResponseStream streamFor(final String data) throws UnsupportedEncodingException {
+ return new IMAPResponseStream(new ByteArrayInputStream(data.getBytes("ISO8859-1")));
+ }
+
+ private byte[] bytes(final String data) throws UnsupportedEncodingException {
+ return data.getBytes("ISO8859-1");
+ }
+
+ @Test
+ public void testPlainLineIsReadWithoutTerminator() throws Exception {
+ final IMAPResponseStream stream = streamFor("* OK ready\r\n");
+ assertArrayEquals(bytes("* OK ready"), stream.readData());
+ }
+
+ @Test
+ public void testBareCarriageReturnDropsNoBytes() throws Exception {
+ // regression: the byte after a bare CR was silently discarded
+ final IMAPResponseStream stream = streamFor("a\rZb\r\n");
+ assertArrayEquals(bytes("a\rZb"), stream.readData());
+ }
+
+ @Test
+ public void testCarriageReturnBeforeLineTerminatorIsKept() throws Exception {
+ // regression: CR CR LF used to leave the line unterminated because the
+ // second CR was consumed as look-ahead and never re-examined
+ final IMAPResponseStream stream = streamFor("a\r\r\nnext\r\n");
+ assertArrayEquals(bytes("a\r"), stream.readData());
+ assertArrayEquals(bytes("next"), stream.readData());
+ }
+
+ @Test
+ public void testConsecutiveBareCarriageReturns() throws Exception {
+ final IMAPResponseStream stream = streamFor("a\r\rb\r\n");
+ assertArrayEquals(bytes("a\r\rb"), stream.readData());
+ }
+
+ @Test
+ public void testLiteralProcessingStillWorks() throws Exception {
+ // a literal marker at the end of the line continues onto the next line;
+ // the literal content may contain CR and LF bytes that must be kept verbatim
+ final IMAPResponseStream stream = streamFor("* 1 FETCH (BODY[] {6}\r\nX\rY\nZW)\r\n");
+ assertArrayEquals(bytes("* 1 FETCH (BODY[] {6}\r\nX\rY\nZW)"), stream.readData());
+ }
+
+ @Test
+ public void testMultipleLinesAreSplitCorrectly() throws Exception {
+ final IMAPResponseStream stream = streamFor("first\r\nsecond\r\nthird\r\n");
+ assertArrayEquals(bytes("first"), stream.readData());
+ assertArrayEquals(bytes("second"), stream.readData());
+ assertArrayEquals(bytes("third"), stream.readData());
+ }
+
+ @Test
+ public void testReadResponseParsesTaggedResponse() throws Exception {
+ final IMAPResponseStream stream = streamFor("a1 OK LOGIN completed.\r\n");
+ final IMAPResponse response = stream.readResponse();
+ final IMAPTaggedResponse tagged = (IMAPTaggedResponse) response;
+ assertTrue(tagged.isOK());
+ }
+}
(geronimo-mail) 04/05: GERONIMO-6904 - A garbled IMAP response leaves the connection poolable and poisons later commands
Posted by rz...@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
commit 2392bdffde8de3243f8f1a0468ddf43bd3894bff
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Jul 18 21:10:28 2026 +0200
GERONIMO-6904 - A garbled IMAP response leaves the connection poolable and poisons later commands
---
.../mail/store/imap/connection/IMAPConnection.java | 13 +++-
.../IMAPConnectionBrokenResponseTest.java | 86 ++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
index 1d1192f..cc0ae9f 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
@@ -399,7 +399,18 @@ public class IMAPConnection extends MailConnection {
public IMAPTaggedResponse receiveResponse() throws MessagingException {
while (true) {
// read and parse a response from the server.
- IMAPResponse response = reader.readResponse();
+ IMAPResponse response;
+ try {
+ response = reader.readResponse();
+ } catch (MessagingException e) {
+ // a read or parsing failure leaves the stream position somewhere in the
+ // middle of a response, so this connection can no longer be trusted to
+ // stay in sync with the server. Close it down and mark it so it is not
+ // returned to the connection pool, then let the failure propagate.
+ setClosed();
+ closeServerConnection();
+ throw e;
+ }
// The response set is terminated by either a continuation response or a
// tagged response (we only have a single command active at one time).
if (response instanceof IMAPTaggedResponse) {
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnectionBrokenResponseTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnectionBrokenResponseTest.java
new file mode 100644
index 0000000..1df014c
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnectionBrokenResponseTest.java
@@ -0,0 +1,86 @@
+/**
+ * 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.connection;
+
+import java.io.ByteArrayInputStream;
+import java.util.Properties;
+
+import jakarta.mail.MessagingException;
+import jakarta.mail.Session;
+
+import org.apache.geronimo.mail.util.ProtocolProperties;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression test for the broken-conn-containment defect: when
+ * readResponse() fails (garbled data or unexpected EOF), the stream position
+ * of the connection is undefined mid-response. Such a connection used to
+ * remain poolable, so one garbled exchange could poison a later, unrelated
+ * command. receiveResponse() must mark the connection closed (and shut the
+ * server connection down) before propagating the failure, so the pool
+ * discards it via the releaseClosedConnection path.
+ */
+public class IMAPConnectionBrokenResponseTest {
+
+ private IMAPConnection connectionReading(final byte[] data) {
+ final ProtocolProperties props = new ProtocolProperties(
+ Session.getInstance(new Properties()), "imap", false, 143);
+ final IMAPConnection connection = new IMAPConnection(props, null);
+ // inject the response reader directly; the connection was never
+ // actually connected, so there is no socket to clean up.
+ connection.reader = new IMAPResponseStream(new ByteArrayInputStream(data));
+ return connection;
+ }
+
+ @Test
+ public void testGarbledResponseMarksConnectionClosed() throws Exception {
+ // an untagged response whose payload cannot be parsed ("* )" is the
+ // shape seen in the transient TCK flake)
+ final IMAPConnection connection = connectionReading("* )\r\n".getBytes("ISO8859-1"));
+ assertFalse(connection.isClosed(), "connection must start out poolable");
+
+ assertThrows(MessagingException.class, connection::receiveResponse,
+ "a garbled response must surface as a MessagingException");
+ assertTrue(connection.isClosed(),
+ "a connection that failed mid-response must not be poolable");
+ }
+
+ @Test
+ public void testPrematureEndOfStreamMarksConnectionClosed() throws Exception {
+ // EOF in the middle of a response line
+ final IMAPConnection connection = connectionReading("* OK incomplete".getBytes("ISO8859-1"));
+
+ assertThrows(MessagingException.class, connection::receiveResponse,
+ "an EOF mid-response must surface as a MessagingException");
+ assertTrue(connection.isClosed(),
+ "a connection whose stream hit EOF mid-response must not be poolable");
+ }
+
+ @Test
+ public void testCleanResponseLeavesConnectionPoolable() throws Exception {
+ final IMAPConnection connection = connectionReading("a1 OK completed\r\n".getBytes("ISO8859-1"));
+
+ final IMAPTaggedResponse response = connection.receiveResponse();
+ assertTrue(response.isOK());
+ assertFalse(connection.isClosed(),
+ "a successfully parsed exchange must leave the connection poolable");
+ }
+}
(geronimo-mail) 02/05: GERONIMO-6902 - Folder.expunge() always returns an empty array - EXPUNGE responses extracted under the wrong keyword
Posted by rz...@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
commit b78b1e61c21e9d6dbc22aafdefe6211e1fe1015c
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Jul 18 21:10:28 2026 +0200
GERONIMO-6902 - Folder.expunge() always returns an empty array - EXPUNGE responses extracted under the wrong keyword
---
.../mail/store/imap/connection/IMAPConnection.java | 8 +-
.../mail/store/imap/IMAPExpungeRegressionTest.java | 107 +++++++++++++++++++++
geronimo-mail_2.1_tck/src/tck/geronimo.jtx | 1 -
3 files changed, 113 insertions(+), 3 deletions(-)
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
index 9351115..1d1192f 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
@@ -1359,8 +1359,12 @@ public class IMAPConnection extends MailConnection {
public synchronized List expungeMailbox() throws MessagingException {
// send the message, and make sure we got an OK response
sendCommand("EXPUNGE");
- // extract all of the expunged responses and return.
- return extractResponses("EXPUNGED");
+ // extract all of the expunged responses and return. The parser queues
+ // the untagged "* nn EXPUNGE" responses under the "EXPUNGE" keyword, so
+ // that is the keyword we must extract here. Extracting them prevents the
+ // pending-response handler from processing the same expunge events a
+ // second time.
+ return extractResponses("EXPUNGE");
}
public int[] searchMailbox(SearchTerm term) throws MessagingException {
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPExpungeRegressionTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPExpungeRegressionTest.java
new file mode 100644
index 0000000..84024ba
--- /dev/null
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPExpungeRegressionTest.java
@@ -0,0 +1,107 @@
+/**
+ * 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.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.util.Properties;
+
+import jakarta.mail.Flags;
+import jakarta.mail.Folder;
+import jakarta.mail.Message;
+import jakarta.mail.Session;
+import jakarta.mail.Store;
+
+import org.apache.geronimo.mail.testserver.AbstractProtocolTest;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression test for the expunge-keyword-typo defect surfaced by the
+ * Jakarta Mail TCK (exception#msgRemoveExp_Test):
+ * IMAPConnection.expungeMailbox() extracted the untagged expunge responses
+ * with the keyword "EXPUNGED", but the parser queues them under "EXPUNGE".
+ * As a result Folder.expunge() always returned an empty array and the
+ * unextracted responses were double-processed by the pending-response
+ * handler.
+ */
+public class IMAPExpungeRegressionTest extends AbstractProtocolTest {
+
+ protected 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;
+ }
+
+ protected byte[] readMessageResource(final String name) throws Exception {
+ final ByteArrayOutputStream bout = new ByteArrayOutputStream();
+ try (InputStream in = IMAPExpungeRegressionTest.class.getResourceAsStream(name)) {
+ final byte[] buf = new byte[4096];
+ int n;
+ while ((n = in.read(buf)) != -1) {
+ bout.write(buf, 0, n);
+ }
+ }
+ return bout.toByteArray();
+ }
+
+ @Test
+ public void testExpungeReturnsExpungedMessages() throws Exception {
+ start();
+ server.createUserMailbox("test1");
+ final byte[] message = readMessageResource("/messages/simple.msg");
+ server.appendToUserMailbox("test1", message);
+ server.appendToUserMailbox("test1", message);
+ server.appendToUserMailbox("test1", message);
+
+ final Store store = connect();
+ try {
+ final Folder folder = store.getDefaultFolder().getFolder("test1");
+ folder.open(Folder.READ_WRITE);
+ try {
+ assertEquals(3, folder.getMessageCount(), "test setup must provide three messages");
+
+ // flag two of the three messages for deletion
+ folder.getMessage(1).setFlag(Flags.Flag.DELETED, true);
+ folder.getMessage(3).setFlag(Flags.Flag.DELETED, true);
+
+ final Message[] expunged = folder.expunge();
+ assertNotNull(expunged, "expunge() must never return null");
+ assertEquals(2, expunged.length,
+ "expunge() must return exactly the messages removed from the folder");
+ for (final Message m : expunged) {
+ assertTrue(m.isExpunged(), "returned messages must be marked expunged");
+ }
+
+ // the message cache must have been updated exactly once per expunge
+ assertEquals(1, folder.getMessageCount(),
+ "one message must remain after expunging two of three");
+ } finally {
+ folder.close(false);
+ }
+ } finally {
+ store.close();
+ }
+ }
+}
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index bc723e6..945b564 100644
--- a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
+++ b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
@@ -33,7 +33,6 @@ SignatureTest.html
# IMAPFolder.renameTo fix removed a ~90-test cascade). Distinct defects:
# folder create/delete (topdog), permanent flags, list/listSubscribed,
# several MIME encoding/decoding and unicode behaviours.
-javasoft/sqe/tests/jakarta/mail/exception/testlist.html#msgRemoveExp_Test
javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#getPermanentFlags_Test
javasoft/sqe/tests/jakarta/mail/internet/InternetAddress/testlist.html#unicode_Test
javasoft/sqe/tests/jakarta/mail/internet/MimeBodyPart/testlist.html#attachFile_saveFile_Test