Merge pull request #7801 from thunderbird/imap_open

Change the way `RealImapFolder` checks for open folders
thunderbird · May 2, 2024 · b38580c · b38580c
2 parents 1b54bd4 + 9c79c85
commit b38580c
Show file tree

Hide file tree

Showing 6 changed files with 182 additions and 52 deletions.
diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandExpunge.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandExpunge.kt
@@ -12,7 +12,6 @@ internal class CommandExpunge(private val imapStore: ImapStore) {
         val remoteFolder = imapStore.getFolder(folderServerId)
         try {
             remoteFolder.open(OpenMode.READ_WRITE)
-            if (remoteFolder.mode != OpenMode.READ_WRITE) return
 
             remoteFolder.expunge()
 
@@ -26,7 +25,6 @@ internal class CommandExpunge(private val imapStore: ImapStore) {
         val remoteFolder = imapStore.getFolder(folderServerId)
         try {
             remoteFolder.open(OpenMode.READ_WRITE)
-            if (remoteFolder.mode != OpenMode.READ_WRITE) return
 
             remoteFolder.expungeUids(messageServerIds)
         } finally {

diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandMarkAllAsRead.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandMarkAllAsRead.kt
@@ -10,7 +10,6 @@ internal class CommandMarkAllAsRead(private val imapStore: ImapStore) {
         val remoteFolder = imapStore.getFolder(folderServerId)
         try {
             remoteFolder.open(OpenMode.READ_WRITE)
-            if (remoteFolder.mode != OpenMode.READ_WRITE) return
 
             remoteFolder.setFlagsForAllMessages(setOf(Flag.SEEN), true)
         } finally {

diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandMoveOrCopyMessages.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandMoveOrCopyMessages.kt
@@ -1,7 +1,6 @@
 package com.fsck.k9.backend.imap
 
 import com.fsck.k9.logging.Timber
-import com.fsck.k9.mail.MessagingException
 import com.fsck.k9.mail.store.imap.ImapFolder
 import com.fsck.k9.mail.store.imap.ImapStore
 import com.fsck.k9.mail.store.imap.OpenMode
@@ -42,12 +41,6 @@ internal class CommandMoveOrCopyMessages(private val imapStore: ImapStore) {
             }
 
             remoteSrcFolder.open(OpenMode.READ_WRITE)
-            if (remoteSrcFolder.mode != OpenMode.READ_WRITE) {
-                throw MessagingException(
-                    "moveOrCopyMessages: could not open remoteSrcFolder $srcFolder read/write",
-                    true,
-                )
-            }
 
             val messages = uids.map { uid -> remoteSrcFolder.getMessage(uid) }
 

diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandSetFlag.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandSetFlag.kt
@@ -12,7 +12,6 @@ internal class CommandSetFlag(private val imapStore: ImapStore) {
         val remoteFolder = imapStore.getFolder(folderServerId)
         try {
             remoteFolder.open(OpenMode.READ_WRITE)
-            if (remoteFolder.mode != OpenMode.READ_WRITE) return
 
             val messages = messageServerIds.map { uid -> remoteFolder.getMessage(uid) }
 

diff --git a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt
@@ -40,9 +40,23 @@ internal class RealImapFolder(
     private var canCreateKeywords = false
     private var uidValidity: Long? = null
 
+    /**
+     * Specifies whether the folder was opened in read-only or read-write mode based on the the tagged OK response to
+     * the SELECT or EXAMINE command (READ-ONLY or READ-WRITE).
+     *
+     * Most of the time this will match the [mode] value. But it's possible for the SELECT command to open a folder in
+     * read-only mode. See RFC 3501, section 6.3.1.
+     *
+     * Note: Currently unused.
+     */
+    private var serverOpenMode: OpenMode? = null
+
     override var messageCount = -1
         private set
 
+    /**
+     * The [OpenMode] value that was passed when this folder was [opened][open].
+     */
     override var mode: OpenMode? = null
         private set
 
@@ -84,6 +98,12 @@ internal class RealImapFolder(
         return handleUntaggedResponses(connection!!.executeSimpleCommand(command))
     }
 
+    /**
+     * Opens the folder in either read-only or read-write mode.
+     *
+     * When [OpenMode.READ_ONLY] is passed the EXAMINE command is used to open the folder.
+     * When [OpenMode.READ_WRITE] is passed the SELECT command is used to open the folder.
+     */
     @Throws(MessagingException::class)
     override fun open(mode: OpenMode) {
         internalOpen(mode)
@@ -118,10 +138,6 @@ internal class RealImapFolder(
             val command = String.format("%s %s", openCommand, escapedFolderName)
             val responses = executeSimpleCommand(command)
 
-            /*
-             * If the command succeeds we expect the folder has been opened read-write unless we
-             * are notified otherwise in the responses.
-             */
             this.mode = mode
 
             for (response in responses) {
@@ -160,7 +176,7 @@ internal class RealImapFolder(
     private fun handleSelectOrExamineOkResponse(response: ImapResponse) {
         val selectOrExamineResponse = SelectOrExamineResponse.parse(response) ?: return // This shouldn't happen
         if (selectOrExamineResponse.hasOpenMode()) {
-            mode = selectOrExamineResponse.openMode
+            serverOpenMode = selectOrExamineResponse.openMode
         }
     }
 
@@ -291,7 +307,7 @@ internal class RealImapFolder(
             return null
         }
 
-        checkOpen()
+        checkOpenWithWriteAccess()
 
         return if (connection.hasCapability(Capabilities.MOVE)) {
             moveMessagesUsingMoveExtension(messages, folder)
@@ -964,7 +980,7 @@ internal class RealImapFolder(
      */
     @Throws(MessagingException::class)
     override fun appendMessages(messages: List<Message>): Map<String, String>? {
-        checkOpen()
+        checkOpenWithWriteAccess()
 
         return try {
             val uidMap: MutableMap<String, String> = HashMap()
@@ -1073,7 +1089,7 @@ internal class RealImapFolder(
 
     @Throws(MessagingException::class)
     override fun expunge() {
-        checkOpen()
+        checkOpenWithWriteAccess()
 
         try {
             executeSimpleCommand("EXPUNGE")
@@ -1083,7 +1099,7 @@ internal class RealImapFolder(
     }
 
     override fun expungeUids(uids: List<String>) {
-        checkOpen()
+        checkOpenWithWriteAccess()
         expungeUids(uids, fullExpungeFallback = true)
     }
 
@@ -1110,7 +1126,7 @@ internal class RealImapFolder(
 
     @Throws(MessagingException::class)
     override fun setFlagsForAllMessages(flags: Set<Flag>, value: Boolean) {
-        checkOpen()
+        checkOpenWithWriteAccess()
 
         val canCreateForwardedFlag = canCreateKeywords ||
             internalImapStore.getPermanentFlagsIndex().contains(Flag.FORWARDED)
@@ -1132,7 +1148,7 @@ internal class RealImapFolder(
 
     @Throws(MessagingException::class)
     override fun setFlags(messages: List<ImapMessage>, flags: Set<Flag>, value: Boolean) {
-        checkOpen()
+        checkOpenWithWriteAccess()
 
         val uids = messages.map { it.uid.toLong() }.toSet()
         val canCreateForwardedFlag = canCreateKeywords ||
@@ -1146,11 +1162,13 @@ internal class RealImapFolder(
         }
     }
 
-    @Throws(MessagingException::class)
     private fun checkOpen() {
-        if (!isOpen) {
-            throw MessagingException("Folder $serverId is not open.")
-        }
+        check(isOpen) { "Folder '$serverId' is not open." }
+    }
+
+    private fun checkOpenWithWriteAccess() {
+        checkOpen()
+        check(mode == OpenMode.READ_WRITE) { "Folder '$serverId' needs to be opened for read-write access." }
     }
 
     private fun ioExceptionHandler(connection: ImapConnection?, ioe: IOException): MessagingException {