[Web] fix exception handler and rspamd_maps function

[FreddleSpl0it]

No description provided.

[smarsching]

@FreddleSpl0it

I might be wrong about this, but I think that one of the commits that is part of this PR (3aee2b6) introduces a potential bug. I say potential, because due to circumstances, this bug is not triggered at the moment but lies dormant instead.

The problem is with the fix in data/web/inc/functions.rspamd.inc.php. Before the fix, a map name that did not survive validation would still be processed later. After the fix, this won’t happen, but the fix has the (most likely unintended) consequence, that a valid map name might get processed multiple times, or an error might be reported for a valid map name even though it is processed.

This is the problematic section of the code:

      $maps = (array)$_data['map'];
      $valid_maps = array();
      foreach ($maps as $map) {
        foreach ($RSPAMD_MAPS as $rspamd_map_type) {
          if (!in_array($map, $rspamd_map_type)) {
            $_SESSION['return'][] = array(
              'type' => 'danger',
              'log' => array(__FUNCTION__, $_action, '-'),
              'msg' => array('global_map_invalid', $map)
            );
          } else {
            array_push($valid_maps, $map);
          }
        }
      }

The bug lies dormant because according to data/web/inc/vars.inc.php, $RSPAMD_MAPS currently only has a single entry, so there only is a single iteration of the inner loop.

Consider the case, where a second element is added to $RSPAMD_MAPS: Now, there will be two iterations of the inner loop. If the $map is present in both entries, it is going to be added to $valid_maps twice, so it is also going to be processed twice. If it is only present in one of the two entries, it is going to be added to $valid_maps once, but it will also be added to $_SESSION['return']. If it is not contained in any of the entries (so it actually is invalid), it will be added to $_SESSION['return'] twice.

I think that the correct way to fix this is doing the adding to either of the arrays outside of the inner loop, so the code could look like this:

      $maps = (array)$_data['map'];
      $valid_maps = array();
      foreach ($maps as $map) {
        $map_is_valid = false;
        foreach ($RSPAMD_MAPS as $rspamd_map_type) {
          if (in_array($map, $rspamd_map_type)) {
            $map_is_valid = true;
            break; // break the inner loop
          }
        }
        if ($map_is_valid) {
          array_push($valid_maps, $map);
        } else {
          $_SESSION['return'][] = array(
            'type' => 'danger',
            'log' => array(__FUNCTION__, $_action, '-'),
            'msg' => array('global_map_invalid', $map)
          );
        }
      }

This way, $map is going to be added to $valid_maps exactly once if it is present in any of the entries in $RSPAMD_MAPS. Otherwise, the error message is going to be added to $_SESSION['return'] exactly once.

[FreddleSpl0it]

@smarsching You are correct, thank you for pointing this out. I will prepare a PR for this.

[FreddleSpl0it]

@smarsching see 7660ca8

[smarsching]

@FreddleSpl0it Looks good now. Thanks for fixing it so quickly.