Skip to content

Managesieve: preserve an explicitly-selected i;ascii-casemap comparator (#9981) - #10266

Open
MiMoHo wants to merge 1 commit into
roundcube:masterfrom
MiMoHo:pr-managesieve-casemap-9981
Open

Managesieve: preserve an explicitly-selected i;ascii-casemap comparator (#9981)#10266
MiMoHo wants to merge 1 commit into
roundcube:masterfrom
MiMoHo:pr-managesieve-casemap-9981

Conversation

@MiMoHo

@MiMoHo MiMoHo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

When a user explicitly selects the i;ascii-casemap comparator for a filter test, the choice is silently dropped from the generated Sieve script. The saved rule therefore does not reflect the explicit selection, and re-parsing/re-saving the script loses it entirely (round-trip loss). See #9981.

Root cause

plugins/managesieve/lib/Roundcube/rcube_sieve_script.php, add_comparator() (previously ~line 1056):

// skip default comparator
if ($test['comparator'] != 'i;ascii-casemap') {
    $out .= ' :comparator ' . self::escape_string($test['comparator']);
}

Because i;ascii-casemap is the RFC 5228 default comparator, the code unconditionally suppressed it on output. However, both the parser (test_tokens()) and the engine (rcube_sieve_engine.php:1015, only sets comparator when the user picked one) populate the comparator key only on an explicit selection — an unset comparator leaves the key empty and is already short-circuited by the empty() guard at the top of add_comparator(). So the presence of comparator == 'i;ascii-casemap' unambiguously means "the user chose it", yet it was still discarded.

Fix

Emit the comparator whenever it is present. The existing empty() guard still means the default (unset) case emits nothing, so scripts without an explicit comparator are unchanged. Behavior for i;octet and i;ascii-numeric is unchanged, and i;ascii-casemap continues to be excluded from the require/extensions list (it is a built-in comparator requiring no capability).

Testing

Added two focused tests in plugins/managesieve/tests/ManagesieveScriptTest.php:

  • test_explicit_ascii_casemap_comparator — parses a rule with an explicit :comparator "i;ascii-casemap", asserts as_text() emits it, and asserts it survives a re-parse round-trip. Fails on current master, passes after the fix.
  • test_default_comparator_not_emitted — asserts that a rule with no explicit comparator emits no :comparator token (guards against regression).

The existing parser/parser.out fixture was updated to reflect the corrected (non-lossy) output for its explicit i;ascii-casemap case. Full ManagesieveScriptTest suite passes (35 tests). phpstan level 4 on the changed file reports no errors.

Fixes #9981

…or (roundcube#9981)

When a user explicitly selected the "i;ascii-casemap" comparator for a
filter test, it was dropped from the generated script because it equals
the RFC default and add_comparator() skipped emitting it. The explicit
choice was therefore lost, and scripts that contained
:comparator "i;ascii-casemap" did not survive a load/save round-trip.

The comparator is only populated when it was set explicitly - the form
leaves it empty for the "default" option, and the parser only sets it
from an explicit :comparator token - so emitting it whenever present is
safe and does not add a redundant :comparator to ordinary rules. The
parser fixture is updated to reflect the now-preserved comparator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@MiMoHo
MiMoHo force-pushed the pr-managesieve-casemap-9981 branch from ba02adf to f31c058 Compare July 16, 2026 08:13
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.

Managesieve plugin - ignores ascii-casemap comparison

1 participant