From 2db0370bbab37c1514c7b9c6c3c6f01af4aa3e01 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <machniak@kolabsys.com>
Date: Thu, 14 Apr 2016 11:42:37 +0200
Subject: [PATCH] Properly set skip_deleted and threading on Kolab storage IMAP
 operations (T1145)

Summary:
Fixes race-conditions between Kolab folders and Roundcube core
where skip_deleted/threading could be set for operations outside
of Kolab plugins, causing e.g. inability to see \Deleted messages.

Fixes T1145.

Reviewers: #roundcube_kolab_plugins_developers, vanmeeuwen

Reviewed By: #roundcube_kolab_plugins_developers, vanmeeuwen

Maniphest Tasks: T1145

Differential Revision: https://git.kolab.org/D112
---
 plugins/libkolab/lib/kolab_storage.php        |  6 +--
 plugins/libkolab/lib/kolab_storage_cache.php  | 48 +++++++++++++++----
 plugins/libkolab/lib/kolab_storage_folder.php | 25 +++++-----
 3 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/plugins/libkolab/lib/kolab_storage.php b/plugins/libkolab/lib/kolab_storage.php
index 084bcc76..da74c007 100644
--- a/plugins/libkolab/lib/kolab_storage.php
+++ b/plugins/libkolab/lib/kolab_storage.php
@@ -87,11 +87,7 @@ class kolab_storage
             (self::$imap->get_capability('METADATA') || self::$imap->get_capability('ANNOTATEMORE') || self::$imap->get_capability('ANNOTATEMORE2'));
 
         if (self::$ready) {
-            // set imap options
-            self::$imap->set_options(array(
-                'skip_deleted' => true,
-                'threading' => false,
-            ));
+            // do nothing
         }
         else if (!class_exists('kolabformat')) {
             rcube::raise_error(array(
diff --git a/plugins/libkolab/lib/kolab_storage_cache.php b/plugins/libkolab/lib/kolab_storage_cache.php
index bcc9ac3f..5ed3fc3e 100644
--- a/plugins/libkolab/lib/kolab_storage_cache.php
+++ b/plugins/libkolab/lib/kolab_storage_cache.php
@@ -185,7 +185,9 @@ class kolab_storage_cache
 
         if (!$this->ready) {
             // kolab cache is disabled, synchronize IMAP mailbox cache only
+            $this->imap_mode(true);
             $this->imap->folder_sync($this->folder->name);
+            $this->imap_mode(false);
         }
         else {
             // read cached folder metadata
@@ -203,7 +205,7 @@ class kolab_storage_cache
                 $this->_sync_lock();
 
                 // disable messages cache if configured to do so
-                $this->bypass(true);
+                $this->imap_mode(true);
 
                 // synchronize IMAP mailbox cache
                 $this->imap->folder_sync($this->folder->name);
@@ -211,6 +213,8 @@ class kolab_storage_cache
                 // compare IMAP index with object cache index
                 $imap_index = $this->imap->index($this->folder->name, null, null, true, true);
 
+                $this->imap_mode(false);
+
                 // determine objects to fetch or to invalidate
                 if (!$imap_index->is_error()) {
                     $imap_index = $imap_index->get();
@@ -260,8 +264,6 @@ class kolab_storage_cache
                     }
                 }
 
-                $this->bypass(false);
-
                 // remove lock
                 $this->_sync_unlock();
             }
@@ -564,6 +566,8 @@ class kolab_storage_cache
         else {
             $filter = $this->_query2assoc($query);
 
+            $this->imap_mode(true);
+
             if ($filter['type']) {
                 $search = 'UNDELETED HEADER X-Kolab-Type ' . kolab_format::KTYPE_PREFIX . $filter['type'];
                 $index  = $this->imap->search_once($this->folder->name, $search);
@@ -572,6 +576,8 @@ class kolab_storage_cache
                 $index = $this->imap->index($this->folder->name, null, null, true, true);
             }
 
+            $this->imap_mode(false);
+
             if ($index->is_error()) {
                 $this->check_error();
                 if ($uids) {
@@ -631,6 +637,8 @@ class kolab_storage_cache
         else {
             $filter = $this->_query2assoc($query);
 
+            $this->imap_mode(true);
+
             if ($filter['type']) {
                 $search = 'UNDELETED HEADER X-Kolab-Type ' . kolab_format::KTYPE_PREFIX . $filter['type'];
                 $index  = $this->imap->search_once($this->folder->name, $search);
@@ -639,6 +647,8 @@ class kolab_storage_cache
                 $index = $this->imap->index($this->folder->name, null, null, true, true);
             }
 
+            $this->imap_mode(false);
+
             if ($index->is_error()) {
                 $this->check_error();
                 return null;
@@ -1092,13 +1102,31 @@ class kolab_storage_cache
     }
 
     /**
-     * Bypass Roundcube messages cache.
-     * Roundcube cache duplicates information already stored in kolab_cache.
+     * Set Roundcube storage options and bypass messages cache.
      *
-     * @param bool $disable True disables, False enables messages cache
+     * We use skip_deleted and threading settings specific to Kolab,
+     * we have to change these global settings only temporarily.
+     * Roundcube cache duplicates information already stored in kolab_cache,
+     * that's why we can disable it for better performance.
+     *
+     * @param bool $force True to start Kolab mode, False to stop it.
      */
-    public function bypass($disable = false)
+    public function imap_mode($force = false)
     {
+        // remember current IMAP settings
+        if ($force) {
+            $this->imap_options = array(
+                'skip_deleted' => $this->imap->get_option('skip_deleted'),
+                'threading'    => $this->imap->get_threading(),
+            );
+        }
+
+        // re-set IMAP settings
+        $this->imap->set_threading($force ? false : $this->imap_options['threading']);
+        $this->imap->set_options(array(
+                'skip_deleted' => $force ? true : $this->imap_options['skip_deleted'],
+        ));
+
         // if kolab cache is disabled do nothing
         if (!$this->enabled) {
             return;
@@ -1114,7 +1142,7 @@ class kolab_storage_cache
 
         if ($messages_cache) {
             // handle recurrent (multilevel) bypass() calls
-            if ($disable) {
+            if ($force) {
                 $this->cache_bypassed += 1;
                 if ($this->cache_bypassed > 1) {
                     return;
@@ -1130,7 +1158,7 @@ class kolab_storage_cache
             switch ($cache_bypass) {
                 case 2:
                     // Disable messages cache completely
-                    $this->imap->set_messages_caching(!$disable);
+                    $this->imap->set_messages_caching(!$force);
                     break;
 
                 case 1:
@@ -1138,7 +1166,7 @@ class kolab_storage_cache
                     // Default mode is both (MODE_INDEX | MODE_MESSAGE)
                     $mode = rcube_imap_cache::MODE_INDEX;
 
-                    if (!$disable) {
+                    if (!$force) {
                         $mode |= rcube_imap_cache::MODE_MESSAGE;
                     }
 
diff --git a/plugins/libkolab/lib/kolab_storage_folder.php b/plugins/libkolab/lib/kolab_storage_folder.php
index 9975beda..3a808a89 100644
--- a/plugins/libkolab/lib/kolab_storage_folder.php
+++ b/plugins/libkolab/lib/kolab_storage_folder.php
@@ -50,7 +50,6 @@ class kolab_storage_folder extends kolab_storage_folder_api
     function __construct($name, $type = null, $type_annotation = null)
     {
         parent::__construct($name);
-        $this->imap->set_options(array('skip_deleted' => true));
         $this->set_folder($name, $type, $type_annotation);
     }
 
@@ -468,9 +467,9 @@ class kolab_storage_folder extends kolab_storage_folder_api
 
         $this->imap->set_folder($folder);
 
-        $this->cache->bypass(true);
+        $this->cache->imap_mode(true);
         $message = new rcube_message($msguid);
-        $this->cache->bypass(false);
+        $this->cache->imap_mode(false);
 
         // Message doesn't exist?
         if (empty($message->headers)) {
@@ -717,9 +716,9 @@ class kolab_storage_folder extends kolab_storage_folder_api
 
                 if ($old_uid) {
                     // delete old message
-                    $this->cache->bypass(true);
+                    $this->cache->imap_mode(true);
                     $this->imap->delete_message($old_uid, $object['_mailbox']);
-                    $this->cache->bypass(false);
+                    $this->cache->imap_mode(false);
                 }
 
                 // insert/update message in cache
@@ -815,7 +814,7 @@ class kolab_storage_folder extends kolab_storage_folder_api
         $msguid = is_array($object) ? $object['_msguid'] : $this->cache->uid2msguid($object);
         $success = false;
 
-        $this->cache->bypass(true);
+        $this->cache->imap_mode(true);
 
         if ($msguid && $expunge) {
             $success = $this->imap->delete_message($msguid, $this->name);
@@ -824,7 +823,7 @@ class kolab_storage_folder extends kolab_storage_folder_api
             $success = $this->imap->set_flag($msguid, 'DELETED', $this->name);
         }
 
-        $this->cache->bypass(false);
+        $this->cache->imap_mode(false);
 
         if ($success) {
             $this->cache->set($msguid, false);
@@ -844,9 +843,9 @@ class kolab_storage_folder extends kolab_storage_folder_api
         }
 
         $this->cache->purge();
-        $this->cache->bypass(true);
+        $this->cache->imap_mode(true);
         $result = $this->imap->clear_folder($this->name);
-        $this->cache->bypass(false);
+        $this->cache->imap_mode(false);
 
         return $result;
     }
@@ -865,9 +864,9 @@ class kolab_storage_folder extends kolab_storage_folder_api
         }
 
         if ($msguid = $this->cache->uid2msguid($uid, true)) {
-            $this->cache->bypass(true);
+            $this->cache->imap_mode(true);
             $result = $this->imap->set_flag($msguid, 'UNDELETED', $this->name);
-            $this->cache->bypass(false);
+            $this->cache->imap_mode(false);
 
             if ($result) {
                 return $msguid;
@@ -895,9 +894,9 @@ class kolab_storage_folder extends kolab_storage_folder_api
             $target_folder = kolab_storage::get_folder($target_folder);
 
         if ($msguid = $this->cache->uid2msguid($uid)) {
-            $this->cache->bypass(true);
+            $this->cache->imap_mode(true);
             $result = $this->imap->move_message($msguid, $target_folder->name, $this->name);
-            $this->cache->bypass(false);
+            $this->cache->imap_mode(false);
 
             if ($result) {
                 $this->cache->move($msguid, $uid, $target_folder);