diff --git a/CHANGELOG b/CHANGELOG index 0ba4ead6f..d0f5b1b5b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,6 +13,8 @@ CHANGELOG Roundcube Webmail - Archive: Added options to split archive by year or year+month and folder (#7216) - Managesieve: Allow display name with email address in vacation :from field (#6760) - Managesieve: Improve UX on custom header input (#7207) +- Managesieve: Fix bug where activation of forward/vacation rule could activate a wrong script (#7423) +- Managesieve: Fix bug where forward/vacation rule could end up being duplicated (#7349) - Password: Added 'pwned' password strength driver (#7274) - Add support for SameSite cookie attribute via session_samesite option (req PHP >= 7.3.0) (#6772) - Elastic: Display a special icon for other users and shared namespace roots (#5012) diff --git a/plugins/managesieve/Changelog b/plugins/managesieve/Changelog index af4765c32..d13d517e0 100644 --- a/plugins/managesieve/Changelog +++ b/plugins/managesieve/Changelog @@ -4,6 +4,8 @@ - Fix so modifier type select wasn't hidden after hiding modifier select on header change - Fix filter selection after removing a first filter (#7079) - Fix bug where it wasn't possible to save flag actions (#7188) +- Fix bug where activation of forward/vacation rule could activate a wrong script (#7423) +- Fix bug where forward/vacation rule could end up being duplicated (#7349) * version 9.3 [2019-04-21] ----------------------------------------------------------- diff --git a/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php b/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php index eb57a0ad7..e64b74189 100644 --- a/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php +++ b/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php @@ -248,25 +248,9 @@ class rcube_sieve_engine else if ($list) { $script_name = $list[0]; } - // create a new (initial) script else { - // if script not exists build default script contents - $script_file = $this->rc->config->get('managesieve_default'); - $script_name = $this->rc->config->get('managesieve_script_name'); - - if (empty($script_name)) { - $script_name = 'roundcube'; - } - - if ($script_file && is_readable($script_file) && !is_dir($script_file)) { - $content = file_get_contents($script_file); - } - - // add script and set it active - if ($this->sieve->save_script($script_name, $content)) { - $this->activate_script($script_name); - $this->list[] = $script_name; - } + // if script does not exist create one with default content + $this->create_default_script(); } } @@ -3126,4 +3110,135 @@ class rcube_sieve_engine return $select_comp->show($comparator); } + + /** + * Merge a rule into the script + */ + protected function merge_rule($rule, $existing, &$script_name = null) + { + // if script does not exist create a new one + if ($script_name === null || $script_name === false) { + $script_name = $this->create_default_script(); + $this->sieve->load($script_name); + $this->init_script(); + } + + if (!$this->sieve->script) { + return false; + } + + $script_active = in_array($script_name, $this->active); + $rule_active = empty($rule['disabled']); + $activate_script = false; + + // If the script is not active, but the rule is, + // put the rule in an active script if there is one + if (!$script_active && $rule_active && !empty($this->active)) { + // Remove the rule from current (inactive) script + if (isset($existing['idx'])) { + unset($this->script[$existing['idx']]); + $this->sieve->script->content = $this->script; + $this->save_script($script_name); + } + + // Load and init the active script, add the rule there + $this->sieve->load($script_name = $this->active[0]); + $this->init_script(); + array_unshift($this->script, $rule); + } + // update original forward rule/script + else { + // re-order rules if needed + if (isset($rule['after']) && $rule['after'] !== '') { + // unset the original rule + if (isset($existing['idx'])) { + $this->script[$existing['idx']] = null; + } + + // add at target position + if ($rule['after'] >= count($this->script) - 1) { + $this->script[] = $rule; + $this->script = array_values(array_filter($this->script)); + $rule_index = count($this->script); + } + else { + $script = array(); + + foreach ($this->script as $idx => $r) { + if ($r) { + $script[] = $r; + } + + if ($idx == $rule['after']) { + $script[] = $rule; + $rule_index = count($script); + } + } + + $this->script = $script; + } + } + // rule exists, update it "in place" + else if (isset($existing['idx'])) { + $this->script[$existing['idx']] = $rule; + $rule_index = $existing['idx']; + } + // otherwise put the rule on top + else { + array_unshift($this->script, $rule); + $rule_index = 0; + } + + // if the script is not active, but the rule is, we need to de-activate + // all rules except the forward rule + if (!$script_active && $rule_active) { + $activate_script = true; + foreach ($this->script as $idx => $r) { + if ($idx !== $rule_index) { + $this->script[$idx]['disabled'] = true; + } + } + } + } + + $this->sieve->script->content = $this->script; + + // save the script + $saved = $this->save_script($script_name); + + // activate the script + if ($saved && $activate_script) { + $this->activate_script($script_name); + } + + return $saved; + } + + /** + * Create default script + */ + protected function create_default_script() + { + // if script not exists build default script contents + $script_file = $this->rc->config->get('managesieve_default'); + $script_name = $this->rc->config->get('managesieve_script_name'); + $kolab_master = $this->rc->config->get('managesieve_kolab_master'); + $content = ''; + + if (empty($script_name)) { + $script_name = 'roundcube'; + } + + if ($script_file && !$kolab_master && is_readable($script_file) && !is_dir($script_file)) { + $content = file_get_contents($script_file); + } + + // add script and set it active + if ($this->sieve->save_script($script_name, $content)) { + $this->activate_script($script_name); + $this->list[] = $script_name; + } + + return $script_name; + } } diff --git a/plugins/managesieve/lib/Roundcube/rcube_sieve_forward.php b/plugins/managesieve/lib/Roundcube/rcube_sieve_forward.php index b4fcd4ced..a42c5bcc9 100644 --- a/plugins/managesieve/lib/Roundcube/rcube_sieve_forward.php +++ b/plugins/managesieve/lib/Roundcube/rcube_sieve_forward.php @@ -148,7 +148,7 @@ class rcube_sieve_forward extends rcube_sieve_engine $action = 'copy'; } else if ($act['type'] == 'stop') { - // we might loose information if there rules after the stop + // we might loose information if there are rules after the stop $stop_found = true; } else if ($act['type'] == 'discard') { @@ -207,10 +207,8 @@ class rcube_sieve_forward extends rcube_sieve_engine $date_extension = in_array('date', $this->exts); $forward_tests = (array) $this->forward['tests']; - if ($action == 'redirect' || $action == 'copy') { - if (empty($target) || !rcube_utils::check_email($target)) { - $error = 'noemailwarning'; - } + if (empty($target) || !rcube_utils::check_email($target)) { + $error = 'noemailwarning'; } if (empty($forward_tests)) { @@ -224,18 +222,13 @@ class rcube_sieve_forward extends rcube_sieve_engine $rule['disabled'] = $status == 'off'; $rule['tests'] = $forward_tests; $rule['join'] = $date_extension ? count($forward_tests) > 1 : false; - $rule['actions'] = array(); - $rule['after'] = $after; - - if ($action && $action != 'keep') { - $rule['actions'][] = array( - 'type' => $action == 'discard' ? 'discard' : 'redirect', + $rule['actions'] = array(array( + 'type' => 'redirect', 'copy' => $action == 'copy', - 'target' => $action != 'discard' ? $target : '', - ); - } + 'target' => $target, + )); - if ($this->save_forward_script($rule)) { + if ($this->merge_rule($rule, $this->forward, $this->script_name)) { $this->rc->output->show_message('managesieve.forwardsaved', 'confirmation'); $this->rc->output->send(); } @@ -320,99 +313,6 @@ class rcube_sieve_forward extends rcube_sieve_engine return $out; } - /** - * Saves forward script (adding some variables) - */ - protected function save_forward_script($rule) - { - // if script does not exist create a new one - if ($this->script_name === null || $this->script_name === false) { - $this->script_name = $this->rc->config->get('managesieve_script_name'); - if (empty($this->script_name)) { - $this->script_name = 'roundcube'; - } - - // use default script contents - if (!$this->rc->config->get('managesieve_kolab_master')) { - $script_file = $this->rc->config->get('managesieve_default'); - if ($script_file && is_readable($script_file)) { - $content = file_get_contents($script_file); - } - } - - // create and load script - if ($this->sieve->save_script($this->script_name, $content)) { - $this->sieve->load($this->script_name); - } - } - - $script_active = in_array($this->script_name, $this->active); - - // re-order rules if needed - if (isset($rule['after']) && $rule['after'] !== '') { - // reset original forward rule - if (isset($this->forward['idx'])) { - $this->script[$this->forward['idx']] = null; - } - - // add at target position - if ($rule['after'] >= count($this->script) - 1) { - $this->script[] = $rule; - } - else { - $script = array(); - - foreach ($this->script as $idx => $r) { - if ($r) { - $script[] = $r; - } - - if ($idx == $rule['after']) { - $script[] = $rule; - } - } - - $this->script = $script; - } - - $this->script = array_values(array_filter($this->script)); - } - // update original forward rule if it exists - else if (isset($this->forward['idx'])) { - $this->script[$this->forward['idx']] = $rule; - } - // otherwise put forward rule on top - else { - array_unshift($this->script, $rule); - } - - // if the script was not active, we need to de-activate - // all rules except the forward rule, but only if it is not disabled - if (!$script_active && !$rule['disabled']) { - foreach ($this->script as $idx => $r) { - if (empty($r['actions']) || $r['actions'][0]['type'] != 'forward') { - $this->script[$idx]['disabled'] = true; - } - } - } - - if (!$this->sieve->script) { - return false; - } - - $this->sieve->script->content = $this->script; - - // save the script - $saved = $this->save_script($this->script_name); - - // activate the script - if ($saved && !$script_active && !$rule['disabled']) { - $this->activate_script($this->script_name); - } - - return $saved; - } - /** * API: get forward rule * @@ -472,17 +372,13 @@ class rcube_sieve_forward extends rcube_sieve_engine $rule['disabled'] = isset($data['enabled']) && !$data['enabled']; $rule['tests'] = $forward_tests; $rule['join'] = $date_extension ? count($forward_tests) > 1 : false; - $rule['actions'] = array(); - - if ($data['action'] && $data['action'] != 'keep') { - $rule['actions'][] = array( - 'type' => $data['action'] == 'discard' ? 'discard' : 'redirect', + $rule['actions'] = array(array( + 'type' => 'redirect', 'copy' => $data['action'] == 'copy', - 'target' => $data['action'] != 'discard' ? $data['target'] : '', - ); - } + 'target' => $data['target'], + )); - return $this->save_forward_script($rule); + return $this->merge_rule($rule, $this->forward, $this->script_name); } /** diff --git a/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php b/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php index 52d6f7197..8716f4eb4 100644 --- a/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php +++ b/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php @@ -331,7 +331,7 @@ class rcube_sieve_vacation extends rcube_sieve_engine ); } - if ($this->save_vacation_script($rule)) { + if ($this->merge_rule($rule, $this->vacation, $this->script_name)) { $this->rc->output->show_message('managesieve.vacationsaved', 'confirmation'); $this->rc->output->send(); } @@ -408,7 +408,11 @@ class rcube_sieve_vacation extends rcube_sieve_engine $action->add($this->plugin->gettext('vacation.copy'), 'copy'); } - if ($this->rc->config->get('managesieve_vacation') != 2 && !empty($this->vacation['list'])) { + if ( + $this->rc->config->get('managesieve_vacation') != 2 + && !empty($this->vacation['list']) + && in_array($this->script_name, $this->active) + ) { $after = new html_select(array('name' => 'vacation_after', 'id' => 'vacation_after')); $after->add('---', ''); @@ -529,7 +533,7 @@ class rcube_sieve_vacation extends rcube_sieve_engine $table->add('title', html::label('vacation_interval', $this->plugin->gettext('vacation.interval'))); $table->add(null, html::span('input-group', $interval_txt)); - if ($after) { + if (!empty($after)) { $table->add('title', html::label('vacation_after', $this->plugin->gettext('vacation.after'))); $table->add(null, $after->show($this->vacation['idx'] - 1)); } @@ -645,99 +649,6 @@ class rcube_sieve_vacation extends rcube_sieve_engine return $interval ?: ''; } - /** - * Saves vacation script (adding some variables) - */ - protected function save_vacation_script($rule) - { - // if script does not exist create a new one - if ($this->script_name === null || $this->script_name === false) { - $this->script_name = $this->rc->config->get('managesieve_script_name'); - if (empty($this->script_name)) { - $this->script_name = 'roundcube'; - } - - // use default script contents - if (!$this->rc->config->get('managesieve_kolab_master')) { - $script_file = $this->rc->config->get('managesieve_default'); - if ($script_file && is_readable($script_file)) { - $content = file_get_contents($script_file); - } - } - - // create and load script - if ($this->sieve->save_script($this->script_name, $content)) { - $this->sieve->load($this->script_name); - } - } - - $script_active = in_array($this->script_name, $this->active); - - // re-order rules if needed - if (isset($rule['after']) && $rule['after'] !== '') { - // reset original vacation rule - if (isset($this->vacation['idx'])) { - $this->script[$this->vacation['idx']] = null; - } - - // add at target position - if ($rule['after'] >= count($this->script) - 1) { - $this->script[] = $rule; - } - else { - $script = array(); - - foreach ($this->script as $idx => $r) { - if ($r) { - $script[] = $r; - } - - if ($idx == $rule['after']) { - $script[] = $rule; - } - } - - $this->script = $script; - } - - $this->script = array_values(array_filter($this->script)); - } - // update original vacation rule if it exists - else if (isset($this->vacation['idx'])) { - $this->script[$this->vacation['idx']] = $rule; - } - // otherwise put vacation rule on top - else { - array_unshift($this->script, $rule); - } - - // if the script was not active, we need to de-activate - // all rules except the vacation rule, but only if it is not disabled - if (!$script_active && !$rule['disabled']) { - foreach ($this->script as $idx => $r) { - if (empty($r['actions']) || $r['actions'][0]['type'] != 'vacation') { - $this->script[$idx]['disabled'] = true; - } - } - } - - if (!$this->sieve->script) { - return false; - } - - $this->sieve->script->content = $this->script; - - // save the script - $saved = $this->save_script($this->script_name); - - // activate the script - if ($saved && !$script_active && !$rule['disabled']) { - $this->activate_script($this->script_name); - } - - return $saved; - } - /** * API: get vacation rule * @@ -937,7 +848,7 @@ class rcube_sieve_vacation extends rcube_sieve_engine ); } - return $this->save_vacation_script($rule); + return $this->merge_rule($rule, $this->vacation, $this->script_name); } /**