feat(sync): evict ghost items via deferred synthetic deletions

lib/Horde/ActiveSync/Connector/Exporter/Sync.php

@@ -18,6 +18,7 @@
  *
  * @copyright 2009-2020 Horde LLC (http://www.horde.org)
  * @author    Michael J Rubinsky <mrubinsk@horde.org>
+ * @author    Torben Dannhauer <torben@dannhauer.de>
  * @package   ActiveSync
  */
 class Horde_ActiveSync_Connector_Exporter_Sync extends Horde_ActiveSync_Connector_Exporter_Base
@@ -29,6 +30,15 @@ class Horde_ActiveSync_Connector_Exporter_Sync extends Horde_ActiveSync_Connecto
      */
     protected $_seenObjects = [];
 
+    /**
+     * Server ids of SYNC_FETCH requests that failed with a NotFound error
+     * during the last fetchIds() call. Used by the Sync handler to detect
+     * "ghost" items still present on the client but gone from the server.
+     *
+     * @var array
+     */
+    protected $_failedFetchIds = [];
+
     /**
      * Currently syncing collection.
      *
@@ -303,10 +313,15 @@ public function missingRemove($collection)
     /**
      * Send the SYNC_FETCH response for any items requested by the client.
      *
+     * Fetch requests that fail because the item no longer exists are
+     * answered with STATUS_NOTFOUND and recorded; they can be retrieved
+     * with getFailedFetchIds() after this method returns.
+     *
      * @param array $collection  The collection array.
      */
     public function fetchIds($driver, $collection)
     {
+        $this->_failedFetchIds = [];
         foreach ($collection['fetchids'] as $fetch_id) {
             try {
                 $data = $driver->fetch($collection['serverid'], $fetch_id, $collection);
@@ -328,6 +343,7 @@ public function fetchIds($driver, $collection)
                         $fetch_id
                     )
                 );
+                $this->_failedFetchIds[] = $fetch_id;
                 $this->_encoder->startTag(Horde_ActiveSync::SYNC_FETCH);
                 $this->_encoder->startTag(Horde_ActiveSync::SYNC_SERVERENTRYID);
                 $this->_encoder->content($fetch_id);
@@ -340,6 +356,17 @@ public function fetchIds($driver, $collection)
         }
     }
 
+    /**
+     * Return the server ids of Fetch requests that failed with a NotFound
+     * error during the last fetchIds() call.
+     *
+     * @return array  The list of server ids.
+     */
+    public function getFailedFetchIds()
+    {
+        return $this->_failedFetchIds;
+    }
+
     /**
      * Send the SYNC_FOLDERTYPE node if needed.
      *

lib/Horde/ActiveSync/Folder/Imap.php

@@ -1,3 +1,3 @@
 <?php
 
 /**
@@ -17,6 +17,7 @@
  *
  * @copyright 2012-2020 Horde LLC (http://www.horde.org)
  * @author    Michael J Rubinsky <mrubinsk@horde.org>
+ * @author    Torben Dannhauer <torben@dannhauer.de>
  * @package   ActiveSync
  */
 class Horde_ActiveSync_Folder_Imap extends Horde_ActiveSync_Folder_Base implements Serializable
@@ -116,6 +117,19 @@
      */
     protected $_pingStatus = [];
 
+    /**
+     * UIDs of "ghost" items: mail the client still holds (and e.g. retries
+     * SYNC_FETCH for) although it no longer exists in the IMAP folder and is
+     * untracked in $_messages. The regular change diff engine can never
+     * delete such items again, so they are recorded here and exported as
+     * synthetic deletions through the normal change pipeline.
+     *
+     * @see Horde_ActiveSync_State_Base::getChanges()
+     *
+     * @var array
+     */
+    protected $_ghostUids = [];
+
     /**
      * Set message changes.
      *
@@ -555,6 +569,44 @@
         return $this->_removed;
     }
 
+    /**
+     * Record "ghost" UIDs for deferred eviction from the client.
+     *
+     * @param array $uids  The IMAP UIDs.
+     */
+    public function addGhostUids(array $uids)
+    {
+        $this->_ghostUids = array_values(
+            array_unique(array_merge($this->_ghostUids, $uids))
+        );
+    }
+
+    /**
+     * Return the recorded "ghost" UIDs awaiting eviction.
+     *
+     * @return array  The IMAP UIDs.
+     */
+    public function ghostUids()
+    {
+        return $this->_ghostUids;
+    }
+
+    /**
+     * Forget recorded "ghost" UIDs, e.g. after the eviction deletion was
+     * exported to the client.
+     *
+     * @param array $uids  The IMAP UIDs.
+     */
+    public function removeGhostUids(array $uids)
+    {
+        if (!count($this->_ghostUids)) {
+            return;
+        }
+        $this->_ghostUids = array_values(
+            array_diff($this->_ghostUids, $uids)
+        );
+    }
+
     /**
      * Return the minimum IMAP UID contained in this folder.
      *
@@ -601,6 +653,9 @@
         if (!empty($this->_pingStatus)) {
             $data['ps'] = $this->_pingStatus;
         }
+        if (!empty($this->_ghostUids)) {
+            $data['g'] = $this->_ghostUids;
+        }
 
         return $data;
     }
@@ -627,6 +682,7 @@
             ? (bool) $data['hi']
             : !empty($this->_messages);
         $this->_pingStatus = !empty($data['ps']) ? $data['ps'] : [];
+        $this->_ghostUids = !empty($data['g']) ? $data['g'] : [];
 
         if (!empty($this->_status[self::HIGHESTMODSEQ]) && is_string($this->_messages)) {
             $this->_messages = $this->_fromSequenceString($this->_messages);

lib/Horde/ActiveSync/Request/Sync.php

@@ -1,3 +1,3 @@
 <?php
 
 /**
@@ -768,6 +768,22 @@
 
                     // End SYNC_REPLIES
                     $this->_encoder->endTag();
+
+                    // Self-heal ghost items: a Fetch for a mail that is gone
+                    // from IMAP and untracked in the folder state means the
+                    // client still holds an item the server deleted long ago
+                    // (e.g. removed via IMAP/webmail; some clients never act
+                    // on the Fetch STATUS_NOTFOUND reply and retry forever).
+                    // Record the ghost id in the folder state; the eviction
+                    // deletion is exported through the regular change
+                    // pipeline on a later GetChanges Sync. It MUST NOT be
+                    // emitted in this response: iOS maild asserts (SIGABRT
+                    // crash loop) when a Sync response combines a Fetch
+                    // reply with a Remove command for the same item.
+                    $ghostIds = $this->_getGhostFetchIds($exporter, $collection);
+                    if (count($ghostIds)) {
+                        $this->_state->addGhostUids($ghostIds);
+                    }
                 }
 
                 // Save state
@@ -929,6 +945,61 @@
             && (microtime(true) - $syncOutputStart) >= $budget;
     }
 
+    /**
+     * Return the server ids of failed client Fetch requests that reference
+     * "ghost" items: mail that no longer exists in the IMAP folder and is
+     * unknown to the server's folder state.
+     *
+     * Such items were deleted through another vector (IMAP client, webmail)
+     * and the Remove command sent back then was not (fully) applied by the
+     * client, which now retries the Fetch indefinitely against an item the
+     * change diff engine will never delete again. The caller records these
+     * ids in the folder state for deferred eviction through the regular
+     * change pipeline.
+     *
+     * Items that failed to fetch but are still tracked in the folder state
+     * are NOT reported: their deletion (if any) is detected and sent by the
+     * regular change diff engine.
+     *
+     * @param Horde_ActiveSync_Connector_Exporter_Sync $exporter  The
+     *     exporter that handled the collection's Fetch requests.
+     * @param array $collection     The collection array.
+     *
+     * @return array  The list of ghost server ids to evict.
+     */
+    protected function _getGhostFetchIds(
+        Horde_ActiveSync_Connector_Exporter_Sync $exporter,
+        array $collection
+    ) {
+        if (empty($collection['fetchids'])
+            || $collection['class'] != Horde_ActiveSync::CLASS_EMAIL) {
+            return [];
+        }
+        $failed = $exporter->getFailedFetchIds();
+        if (!count($failed)) {
+            return [];
+        }
+        $folderState = $this->_state->getFolderState();
+        if (!($folderState instanceof Horde_ActiveSync_Folder_Imap)) {
+            return [];
+        }
+        $known = $folderState->messages();
+        $ghosts = [];
+        foreach ($failed as $fetch_id) {
+            if (in_array($fetch_id, $known)) {
+                continue;
+            }
+            $this->_logger->info(sprintf(
+                'SYNC: Fetch for %s in %s failed and the item is untracked in folder state; recording ghost uid for deferred eviction from the client.',
+                $fetch_id,
+                $collection['id']
+            ));
+            $ghosts[] = $fetch_id;
+        }
+
+        return $ghosts;
+    }
+
     /**
      * Import a single client-sent ADD or MODIFY command and record the
      * result in the collection array (replies, failures, atchash, ...).

lib/Horde/ActiveSync/State/Base.php

@@ -295,6 +295,32 @@ protected function _assertValidSyncFolderBeforeSave()
         }
     }
 
+    /**
+     * Return the folder state object loaded for the current collection.
+     *
+     * @return Horde_ActiveSync_Folder_Base|array|null  The folder state, an
+     *         empty array for FOLDERSYNC state, or null if no state loaded.
+     */
+    public function getFolderState()
+    {
+        return $this->_folder ?? null;
+    }
+
+    /**
+     * Record "ghost" UIDs in the loaded IMAP folder state for deferred
+     * eviction: the deletion is exported through the regular change pipeline
+     * on a later GetChanges SYNC (@see getChanges()). No-op for non-email
+     * folder state.
+     *
+     * @param array $uids  The IMAP UIDs.
+     */
+    public function addGhostUids(array $uids)
+    {
+        if ($this->_folder instanceof Horde_ActiveSync_Folder_Imap) {
+            $this->_folder->addGhostUids($uids);
+        }
+    }
+
     /**
      * Update the $oldKey syncState to $newKey.
      *
@@ -831,6 +857,12 @@ public function getChanges(array $options = [])
                 $this->_logger->meta('STATE: No client changes, returning all messages.');
                 $this->_changes = $changes;
             }
+
+            // Evict "ghost" items recorded in the folder state: deletions
+            // the diff engine can never produce again (@see addGhostUids()).
+            // Injected in PING mode too so the client is woken to pick up
+            // the eviction on its next SYNC.
+            $this->_injectGhostUidDeletions();
         } else {
             // FOLDERSYNC changes.
             $this->_getFolderChanges();
@@ -1344,6 +1376,67 @@ protected function _acknowledgeExportedChange($type, array $change)
                     $this->_folder->acknowledgeExportedMessage($change['id']);
                 }
                 break;
+
+            case Horde_ActiveSync::CHANGE_TYPE_DELETE:
+                // A delivered deletion also settles any recorded ghost
+                // eviction for this uid (no-op for regular deletions).
+                if (!empty($change['id'])) {
+                    $this->_folder->removeGhostUids([$change['id']]);
+                }
+                break;
+        }
+    }
+
+    /**
+     * Append synthetic deletions for recorded "ghost" UIDs to the current
+     * change set.
+     *
+     * Ghost items exist only on the client (e.g. a deletion Remove was never
+     * applied and the client retries SYNC_FETCH for the item forever). They
+     * are untracked in the folder state, so the diff engine will never
+     * delete them again; exporting a synthetic deletion through the normal
+     * change pipeline evicts them. The recorded uid is cleared once the
+     * deletion is exported (@see _acknowledgeExportedChange()).
+     *
+     * @see Horde_ActiveSync_Folder_Imap::addGhostUids()
+     */
+    protected function _injectGhostUidDeletions()
+    {
+        if (!$this->_folder instanceof Horde_ActiveSync_Folder_Imap) {
+            return;
+        }
+        $ghosts = $this->_folder->ghostUids();
+        if (!count($ghosts)) {
+            return;
+        }
+        // Initial sync batches are bare uid lists; never mix change hashes
+        // into them. Ghosts are delivered once the structure is normalized.
+        if (count($this->_changes) && !is_array(reset($this->_changes))) {
+            return;
+        }
+        $pending = [];
+        foreach ($this->_changes as $change) {
+            $pending[$change['id']] = true;
+        }
+        $known = $this->_folder->messages();
+        foreach ($ghosts as $uid) {
+            if (!empty($pending[$uid])) {
+                continue;
+            }
+            if (in_array($uid, $known)) {
+                // Tracked (again); the regular diff engine owns this uid.
+                $this->_folder->removeGhostUids([$uid]);
+                continue;
+            }
+            $this->_logger->info(sprintf(
+                'STATE: Injecting deletion for ghost uid %s in %s to evict it from the client.',
+                $uid,
+                $this->_collection['id']
+            ));
+            $this->_changes[] = [
+                'id' => $uid,
+                'type' => Horde_ActiveSync::CHANGE_TYPE_DELETE,
+            ];
         }
     }