fix(imap_client): status gaps on selected mailboxes — missing messages key and null getUid

[TDannhauer]

Fix IMAP status handling on selected mailboxes (PHP 8+ warnings and ActiveSync fatal)

Target: horde/Imap_Client → FRAMEWORK_6_0
PR: #45
Branch: fix/undefined_array_key_messages

Summary

This PR hardens Horde_Imap_Client when mailbox status is incomplete on an already-selected mailbox:

1. PHP 8+ warnings — Stops repeated Undefined array key "messages" when search(), thread(), or related code calls status() but the socket driver omits uncached fields from the result.
2. Fatal error during sync — Stops Call to a member function getUid() on null in Socket::_status() when STATUS_UIDNEXT_FORCE infers UIDNEXT via FETCH but no message is returned (common during ActiveSync / CONDSTORE sync).

Both issues share the same underlying gap: Socket::_status() did not always fall back to an IMAP STATUS command for missing fields on the selected mailbox. The branch adds that fallback consistently and guards callers that assumed status arrays were always complete.

Problem 1: Undefined array key messages (PHP 8+)

Symptom

Application logs show many entries such as:

HORDE PHP ERROR: Undefined array key "messages" [pid … on line 2287 of "…/lib/Horde/Imap/Client/Base.php"]

This often appears during normal mail usage (IMP mailbox list, search, threading) on PHP 8.0+, where reading a non-existent array key emits a warning that Horde logs as an error.

Root cause

1. Base::search() optimizes empty-mailbox handling by calling status() with STATUS_MESSAGES (and sometimes STATUS_HIGHESTMODSEQ), then reads $status_res['messages'] (~line 2287).
2. Socket::_status() treats a selected mailbox differently from an unselected one: if Horde_Imap_Client_Base_Mailbox::getStatus() returns null for a field (e.g. STATUS_MESSAGES not yet set after SELECT/EXAMINE), it only handles special cases (UIDNEXT_FORCE, FIRSTUNSEEN/UNSEEN). For ordinary fields like messages, it neither fills $data nor adds the field to the STATUS command $query.
3. status() then returns an array without a messages key.
4. $status_res['messages'] triggers Undefined array key "messages" on PHP 8+.

The bug is intermittent: it depends on whether the mailbox object already has cached status from a prior SELECT/EXAMINE response.

Affected code paths (Problem 1)

Location	Usage
Base.php ~2256	SEARCH_RESULTS_COUNT optimization for ALL
Base.php ~2287	search() empty-mailbox optimization
Base.php ~2451	thread() empty-mailbox shortcut
Base.php ~3556	resolveIds() ALL sequence optimization

Problem 2: getUid() on null during UIDNEXT / sync (fatal)

Symptom

Horde logs a fatal error (not just a PHP warning), often during ActiveSync mail sync:

HORDE Error: Call to a member function getUid() on null in …/lib/Horde/Imap/Client/Socket.php:1712

Example stack trace:

Horde_Imap_Client_Socket->_status()
  Horde_Imap_Client_Base->status()                    [Base.php ~1744 / ~4088]
  Horde_Imap_Client_Base->_syncStatus()               [Base.php ~3643]
  Horde_Imap_Client_Base->getSyncToken()              [Base.php ~3899]
  Horde_Imap_Client_Base->_getSearchCache()           [Base.php ~2276]
  Horde_Imap_Client_Base->search()                    [Base.php ~3580]
  Horde_Imap_Client_Base->resolveIds()                [Base.php ~2625]
  Horde_Imap_Client_Base->_fetchWrapper() / fetch()   [Base.php ~2510]
  Horde_ActiveSync_Imap_Adapter->_getMailMessages()   [Adapter.php ~1075]
  Horde_ActiveSync_Imap_Adapter->getMessages()
  Horde_Core_ActiveSync_Driver->getMessage()
  … ActiveSync Sync request …

Mobile clients (Outlook, iOS Mail, etc.) trigger this path when syncing folder changes.

Root cause

1. _syncStatus() calls status() with STATUS_UIDNEXT_FORCE (among other flags) to build a sync token (getSyncToken() → _getSearchCache()).
2. For a selected mailbox without cached UIDNEXT, Socket::_status() uses the STATUS_UIDNEXT_FORCE branch: if cached STATUS_MESSAGES is non-zero, it FETCHes the largest message UID and sets uidnext = fetch_uid + 1.
3. Horde_Imap_Client_Fetch_Results::first() returns a fetch object only when exactly one message is in the result; otherwise it returns null (0 or 2+ messages).
4. The code called $fetch_res->first()->getUid() unconditionally → fatal when the fetch returned no message.

Why can FETCH return nothing while STATUS_MESSAGES says there are messages?

• Stale cached STATUS_MESSAGES after expunge or concurrent mailbox changes during sync.
• Race between ActiveSync fetch() / search() and server-side updates.
• Selected mailbox status not refreshed after SELECT/EXAMINE omitted optional fields (including UIDNEXT).

This is a separate failure mode from Problem 1, but the same fix pattern applies: if the optimized local path cannot produce a value, fall back to IMAP STATUS.

Affected code paths (Problem 2)

Location	Usage
Socket.php _status()	STATUS_UIDNEXT_FORCE + FETCH largest UID
Base.php _syncStatus()	Sync token via status(… | STATUS_UIDNEXT_FORCE)
Base.php getSyncToken() / _getSearchCache()	Search cache invalidation on sync
ActiveSync stack	fetch() → resolveIds() → search() during message export

Solution

Socket.php — _status() (both problems)

A. Missing ordinary status fields (Problem 1)

When the mailbox equals $this->_selected and getStatus() returns null for a field that is not handled by UIDNEXT_FORCE or unseen-flag branches, add the field to $query so a STATUS command is issued (same as for unselected mailboxes):

} else {
    /* Status not cached after SELECT/EXAMINE; use STATUS. */
    $query[] = $key;
}

B. Failed UIDNEXT inference via FETCH (Problem 2)

In the STATUS_UIDNEXT_FORCE branch, guard the fetch result and fall back to STATUS when first() is null:

$fetch_res = $this->fetch($this->_selected, $fquery, [
    'ids' => $this->getIdsOb(Horde_Imap_Client_Ids::LARGEST),
]);
if (($first = $fetch_res->first()) !== null) {
    $data[$key] = $first->getUid() + 1;
} else {
    /* Fetch failed to return a message (e.g. stale STATUS_MESSAGES); fall back to STATUS. */
    $query[] = $key;
}

Fetch_Results::first() contract (for reviewers):

// Returns fetch object only if count($this->_data) === 1; otherwise null.
public function first()

Base.php — defensive reads (Problem 1)

Use !empty($status_res['messages']) or $res['messages'] ?? 0 where the code previously assumed messages was always present. A missing key is treated as an empty mailbox (count 0), matching the existing $default_ret branch in search().

Files changed

File	Change
lib/Horde/Imap/Client/Socket.php	Fall back to IMAP STATUS when selected-mailbox status is not cached; guard null first() in STATUS_UIDNEXT_FORCE / FETCH path
lib/Horde/Imap/Client/Base.php	Safe access to messages in search(), thread(), count optimization, and resolveIds()

Commits (branch)

Commit	Description
0a6f7e5	Add fallback to STATUS query for non-cached status fields
523cf06	Handle missing messages in Base.php (?? 0, !empty())
d906639	Handle null fetch result before getUid() in Socket.php

Test plan

Problem 1 (PHP warnings / IMP)

• Reload PHP-FPM / clear opcache after deploy.
• Open IMP (or any app using Horde_Imap_Client): browse INBOX and other folders (including empty folders).
• Confirm logs no longer show Undefined array key "messages" at Base.php:2287 (or related lines).
• Search / filter: Run mail searches (unseen, flagged, custom queries) on folders with and without messages.
• Thread view: Open threaded message list on a non-empty mailbox.
• Empty mailbox: Open or select an empty folder; verify no errors and correct empty UI.
• CONDSTORE: If the server supports CONDSTORE, repeat search/sync after opening a mailbox (exercises STATUS_HIGHESTMODSEQ + STATUS_MESSAGES together).
• Regression: message counts in folder list and unread badges remain correct.

Problem 2 (ActiveSync fatal)

• Configure a mobile device with ActiveSync (Exchange account pointing at Horde).
• Sync mail folders (INBOX, Sent, custom folders); trigger initial sync and incremental sync after new mail arrives.
• While syncing, perform actions that change mailbox state (delete messages, move mail, expunge) and confirm sync continues without getUid() on null in logs.
• Confirm Horde logs no longer show Call to a member function getUid() on null in Socket.php (~1712).
• Regression: sent/received mail, read/unread state, and folder list on the device remain consistent.

Compatibility

• API: Unchanged — status() and search() return shapes are the same; missing messages is now either filled by STATUS or interpreted as 0; missing UIDNEXT is filled by STATUS when the FETCH shortcut fails.
• IMAP: Uses standard STATUS on the selected mailbox when cache is empty or inference fails (RFC 3501 permits STATUS on selected mailboxes).
• PHP: Eliminates PHP 8.0+ warnings and a fatal edge case without changing PHP 7 behavior meaningfully (!empty() / ?? on missing keys).
• Breaking: None.

Deployment note (horde-deployment)

These edits live under vendor/horde/imap_client/. A composer update may overwrite them unless this PR is merged upstream in horde/Imap_Client or applied via composer-patches. Pin or patch the dependency after merge for a durable fix.

Related issues

• Similar PHP 8 Undefined array key noise in Horde IMAP integrations (e.g. Nextcloud Mail with bytestream/horde-imap-client) when status fields are missing or empty() is used on numeric 0 values such as highestmodseq.
• ActiveSync / sync-token paths that call status(… | STATUS_UIDNEXT_FORCE) are especially sensitive to stale STATUS_MESSAGES and empty FETCH results; this PR closes that fatal path without disabling sync optimizations.

[what-the-diff]

PR Summary

• Enhanced Message Count Recording:

  • The update addresses cases when there are no messages returned by the search function, ensuring that it will indicate '0' message count rather than delivering a 'null' response.

• Advanced Verification for Message Status:

  • Modification in the search function and thread function to implement an improved empty-check system. This ensures a more encompassing review of possible returned values related to message status.

• Strengthened Retrieval of Message IDs:

  • Changes were made on how message IDs are fetched specifically on contexts where 'messages' might not be defined. Now, it performs better and safer under diverse conditions.

• More Efficient Error Management in Status Checks:

  • Enhancements made to address scenarios where fetching the first message might not yield anything. This adds a layer of robustness as there is now a plan B - conducting a status query if no message is at hand.

[ralflang]

Potentially closes horde/imp#48