Skip to content

Archive: fix TypeError when archiving all messages via select-all (#10107) - #10263

Open
MiMoHo wants to merge 1 commit into
roundcube:masterfrom
MiMoHo:pr-archive-10107
Open

Archive: fix TypeError when archiving all messages via select-all (#10107)#10263
MiMoHo wants to merge 1 commit into
roundcube:masterfrom
MiMoHo:pr-archive-10107

Conversation

@MiMoHo

@MiMoHo MiMoHo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

Using "select all" and then archiving produces a fatal server error ("Serverfault - moving all Mails in the archive"). On PHP 8 the request dies with:

TypeError: count(): Argument #1 ($value) must be of type Countable|array, string given

Root cause

With select-all, rcmail_action::get_uids() yields the string '*' as $uids (the IMAP wildcard). The archive plugin then calls count() on it in two places:

  • plugins/archive/archive.php:346move_messages_worker() does return count($uids);
  • plugins/archive/archive.php:160move_messages() does $count = count($uids); for the archive-folder-source branch

count() on a string throws a TypeError on PHP 8, aborting the whole archive action.

Fix

Mirror how core handles '*' in program/actions/mail/move.php ($count += is_array($uids) ? count($uids) : 1;): treat a non-array $uids (the '*' wildcard) as a single unit instead of calling count() on it. set_flag() and move_message() already accept '*', so the wildcard is passed through unchanged.

Both count($uids) calls are replaced with is_array($uids) ? count($uids) : 1.

Testing

Added test_move_messages_worker_select_all() in plugins/archive/tests/ArchiveTest.php. It mocks storage so set_flag()/move_message() return true and invokes move_messages_worker('*', ...) via reflection. Before the fix the test fails with the count() TypeError at archive.php:346; after the fix it passes and asserts the correct return value and recorded source/destination folders. Full ArchiveTest.php passes (4 tests) and phpstan (level 4) reports no errors on the changed files.

Fixes #10107

@MiMoHo
MiMoHo force-pushed the pr-archive-10107 branch from aeb8e03 to b220dcc Compare July 16, 2026 08:13
@alecpl

alecpl commented Jul 18, 2026

Copy link
Copy Markdown
Member

The fix is not that simple. $uids can be a string (* or e.g. 1,2,3). We need to count the messages as we do in another code path. Or we change the code so the count is not needed (probably complicated and involves client-side). Using $count = 1 is wrong.

…undcube#10107)

With "select all", rcmail_action::get_uids() yields the string '*' (the
IMAP wildcard) as $uids. move_messages() and move_messages_worker() then
called count() on it, which throws a TypeError on PHP 8 ("count():
Argument roundcube#1 must be of type Countable|array, string given") and aborts
the whole archive action with a server error.

Treat a non-array $uids (the '*' wildcard) as a single unit, matching how
core handles it in program/actions/mail/move.php
($count += is_array($uids) ? count($uids) : 1;). set_flag() and
move_message() already accept '*', so the wildcard is passed through
unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@MiMoHo
MiMoHo force-pushed the pr-archive-10107 branch from b220dcc to 0c5e439 Compare July 19, 2026 02:24
@MiMoHo

MiMoHo commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you're right — count() on the * string was the wrong approach. Reworked it to resolve '*' to the actual UID list via $storage->index(), the same way the archive_type branch does a few lines below, so move_messages_worker() always receives a real array and returns the true number of moved messages. The already-in-archive skip branch uses $storage->index($mbox)->count() for select-all. Updated the test to assert the real count; phpunit + phpstan clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Serverfault - moving all Mails in the archive

2 participants