(geronimo-mail) 07/07: GERONIMO-6901 - Failed Folder.open() leaks the pooled connection, breaks Store.close() and hides FolderNotFoundException

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 38a455a56b38078fe6fcf4fbf461e9ff89abf3d7
Author: Richard Zowalla <[email protected]>
AuthorDate: Sat Jul 18 20:48:28 2026 +0200

    GERONIMO-6901 - Failed Folder.open() leaks the pooled connection, breaks Store.close() and hides FolderNotFoundException
---
 .../geronimo/mail/store/imap/IMAPFolder.java       | 58 ++++++++++++++++++++--
 .../apache/geronimo/mail/store/imap/IMAPStore.java |  4 ++
 .../mail/store/imap/IMAPTckRegressionTest.java     | 33 ++++++++++++
 geronimo-mail_2.1_tck/src/tck/geronimo.jtx         |  1 -
 4 files changed, 90 insertions(+), 6 deletions(-)

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 a362a78..52dfcbc 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
@@ -35,6 +35,7 @@ import jakarta.mail.search.FlagTerm;
 import jakarta.mail.search.SearchTerm;
 
 import org.apache.geronimo.mail.store.imap.connection.*;
+import org.apache.geronimo.mail.util.CommandFailedException;
 
 /**
  * The base IMAP implementation of the javax.mail.Folder
@@ -652,8 +653,28 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
 
 
             try {
-                // try to open, which gives us a lot of initial mailbox state.
-                IMAPMailboxStatus status = currentConnection.openMailbox(fullname, mode == Folder.READ_ONLY);
+                IMAPMailboxStatus status;
+                try {
+                    // try to open, which gives us a lot of initial mailbox state.
+                    status = currentConnection.openMailbox(fullname, mode == Folder.READ_ONLY);
+                } catch (CommandFailedException e) {
+                    // The SELECT/EXAMINE was answered with a tagged NO.  The most likely
+                    // reason is that the mailbox does not exist (or is not selectable),
+                    // which the spec requires us to report as a FolderNotFoundException.
+                    // Verify via a LIST on the same connection.
+                    listInfo = null;
+                    boolean missing;
+                    try {
+                        missing = !checkExistance(currentConnection);
+                    } catch (MessagingException e2) {
+                        // can't tell...report the original failure below.
+                        missing = false;
+                    }
+                    if (missing) {
+                        throw new FolderNotFoundException(this, "Folder " + fullname + " not found on server");
+                    }
+                    throw e;
+                }
 
                 // not available in the requested mode?
                 if (status.mode != mode) {
@@ -686,9 +707,36 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
                 folderOpen = true;
                 notifyConnectionListeners(ConnectionEvent.OPENED);
             } finally {
-                // NB:  this doesn't really release this, but it does drive
-                // the processing of any unsolicited responses.
-                releaseConnection(currentConnection);
+                if (folderOpen) {
+                    // NB:  this doesn't really release this, but it does drive
+                    // the processing of any unsolicited responses.
+                    releaseConnection(currentConnection);
+                }
+                else {
+                    // The open failed.  The folder was registered with the Store and the
+                    // connection reserved before the SELECT/EXAMINE was attempted, so we
+                    // must fully unwind here:  drain any pending unsolicited responses,
+                    // deregister as a response handler, and return the connection to the
+                    // pool (which also removes us from the Store's open-folder list).
+                    // Failing to do so leaks the pooled connection and leaves the Store
+                    // thinking this folder is open, so a later Store.close() would call
+                    // Folder.close() on a closed folder and die with IllegalStateException.
+                    IMAPConnection conn = currentConnection;
+                    currentConnection = null;
+                    if (conn != null) {
+                        try {
+                            conn.processPendingResponses();
+                        } catch (MessagingException e) {
+                            // ignore...we're already on a failure path here.
+                        }
+                        conn.removeResponseHandler(this);
+                        try {
+                            ((IMAPStore)store).releaseFolderConnection(this, conn);
+                        } catch (MessagingException e) {
+                            // ignore...we're already on a failure path here.
+                        }
+                    }
+                }
             }
         }
 	}
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPStore.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPStore.java
index 0502e65..e4a5691 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPStore.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPStore.java
@@ -482,6 +482,10 @@ public class IMAPStore extends Store implements QuotaAwareStore, IMAPUntaggedRes
                 try {
                     folder.close(false);
                 } catch (MessagingException e) {
+                } catch (IllegalStateException e) {
+                    // the folder was never successfully opened (or was already
+                    // closed).  Store.close() must never be aborted by a single
+                    // folder in a bad state.
                 }
             }
         }
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
index b9bfc46..780219f 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
@@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit;
 
 import jakarta.mail.Flags;
 import jakarta.mail.Folder;
+import jakarta.mail.FolderNotFoundException;
 import jakarta.mail.Message;
 import jakarta.mail.Session;
 import jakarta.mail.Store;
@@ -36,6 +37,7 @@ import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -199,6 +201,37 @@ public class IMAPTckRegressionTest extends AbstractProtocolTest {
         }
     }
 
+    /**
+     * exception#folderNotFoundExp_Test: opening a nonexistent folder must throw
+     * FolderNotFoundException (not the raw CommandFailedException of the failed
+     * SELECT/EXAMINE).  In addition, the failed open used to leave the folder
+     * registered as open in the Store and leak the pooled connection, so a
+     * subsequent Store.close() blew up with an IllegalStateException.
+     */
+    @Test
+    public void testOpenNonexistentFolderThrowsFolderNotFoundAndStoreStillCloses() throws Exception {
+        start();
+
+        final Store store = connect();
+        try {
+            final Folder folder = store.getDefaultFolder().getFolder("noSuchFolder");
+            assertThrows(FolderNotFoundException.class, () -> folder.open(Folder.READ_ONLY),
+                    "opening a nonexistent folder must throw FolderNotFoundException");
+            assertFalse(folder.isOpen(), "the folder must not claim to be open after a failed open");
+
+            // a folder that really exists must still be openable afterwards,
+            // proving the pooled connection was returned in a usable state
+            createMailboxWithMessage("test1");
+            final Folder good = store.getDefaultFolder().getFolder("test1");
+            good.open(Folder.READ_ONLY);
+            assertTrue(good.getMessageCount() == 1);
+            good.close(false);
+        } finally {
+            // must not throw IllegalStateException because of the failed open
+            store.close();
+        }
+    }
+
     /**
      * MimeMessage#getContentLanguage_Test: a message whose BODYSTRUCTURE
      * carries no language information used to trigger a NullPointerException;
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index 95b54cc..bc723e6 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#folderNotFoundExp_Test
 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