Fix deleting all folder messages on 'select all' when the search context is lost#10255
Fix deleting all folder messages on 'select all' when the search context is lost#10255TowyTowy wants to merge 1 commit into
Conversation
alecpl
left a comment
alecpl
left a comment
There was a problem hiding this comment.
We have the same issue in delete, move and mark actions too (maybe more). So, I think we should move the check to index.php and bail out whenever search is requested but does not exist, and _action is not empty (or if it is an ajax action, I'm not sure which would be better).
| // has been overwritten by a search in another browser tab). Otherwise | ||
| // the '*' would resolve to the whole folder and delete all its messages | ||
| // instead of only the search result (#9671). | ||
| if ($search_request && $_POST['_uid'] === '*' && !$rcmail->storage->get_search_set()) { |
There was a problem hiding this comment.
The second condition can be removed. It does not matter what _uid is.
…le folder (roundcube#9671) When the client sends a _search request parameter but the referenced search context does not exist in the session anymore (e.g. it has been overwritten by a search in another browser tab), mail actions fall back to operating on the whole folder. E.g. deleting a search result with "select all" (_uid=*) resolves '*' against the folder instead of the search set and deletes every message in the folder. Add a central guard in rcmail_action_mail_index::run(), which runs before every mail action, that bails out with an error whenever a search context is requested but not available and _action is not empty. This covers delete, move, mark and all other actions operating on a search result. Actions where _search is not a message-search reference (autocomplete, list-contacts, search-contacts), periodic check-recent/ refresh requests that already degrade gracefully, and compose are exempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Moved the check into |
6585a58 to
3592f84
Compare
2 participants
Closes #9671.
Problem
When a search result is deleted using "select all", the client sends
_uid=*together with_search=<id>. The delete handler resolves*against the storage search set. If the search context is no longer in the session — e.g. it was overwritten by a new search in another browser tab —rcmail_action_mail_index::run()does not callset_search_set(), so the storage object has no search set.rcube_storage::parse_uids('*')then falls back to1:*and every message in the folder is deleted instead of only the (now lost) search result.This matches the analysis in #9671:
Fix
Bail out with an error in the delete handler when a
_uid=*request is bound to a search (_searchpresent) but no search set exists on the storage object. Single-message and explicit-UID deletes are unaffected, and a normal "select all" in a non-search folder view (no_search) still works.Tests
Added a regression test in
tests/Actions/Mail/DeleteTest.phpthat mocks a missing search set and asserts the action aborts without callingdelete_message(). It fails on current master (the un-guarded code callsdelete_message('*'), i.e. the whole-folder delete) and passes with this change. CHANGELOG entry included.To reproduce manually: open a search result with more than one message; in a second tab run a different search (overwriting the session search state); back in the first tab, Select all → Delete. Without this change the whole folder is emptied.
Disclosure: this fix was prepared with the assistance of an AI coding agent (Claude) and reviewed before submission.