(geronimo-mail) 05/05: GERONIMO-6905 - IMAP response codes in brackets (PERMANENTFLAGS, UIDVALIDITY, UIDNEXT, UNSEEN) are never parsed

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 <[email protected]>
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