From ada4d9a073c53b262b48a2f989289501a73fa834 Mon Sep 17 00:00:00 2001
From: Jared Hancock <jared@osticket.com>
Date: Tue, 30 Dec 2014 14:34:37 -0600
Subject: [PATCH] filters: Fix several small, major issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

  * Fix incorrect mapping to user email address
  * Fix early rejecting of tickets — even if a filter earlier in the
    matching filter list had "stop on match" set
  * Fix ::stopOnMatch referring to incorrect db field

The new logic abandons the early rejection logic in ticket create. Instead,
the normal validation is completed as usual. Thereafter, the filter is
initialized and applied to the ticket. Upon rejection, a RejectedException
is thrown by the ::apply() method of the TicketFilter. The Ticket::create()
method will handle the exception and reject the ticket.
---
 include/class.filter.php | 44 +++++++++++++++++++++-------------------
 include/class.ticket.php | 22 ++++++++++----------
 include/class.user.php   |  3 ++-
 3 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/include/class.filter.php b/include/class.filter.php
index d30a6c68b..06e5fa129 100644
--- a/include/class.filter.php
+++ b/include/class.filter.php
@@ -137,7 +137,7 @@ class Filter {
     }
 
     function stopOnMatch() {
-        return ($this->ht['stop_on_match']);
+        return ($this->ht['stop_onmatch']);
     }
 
     function matchAllRules() {
@@ -738,7 +738,7 @@ class TicketFilter {
         $res = $this->getAllActive();
         if($res) {
             while (list($id) = db_fetch_row($res))
-                array_push($this->filters, new Filter($id));
+                $this->filters[] = new Filter($id);
         }
 
         return $this->filters;
@@ -764,36 +764,25 @@ class TicketFilter {
 
         return $this->short_list;
     }
-    /**
-     * Determine if the filters that match the received vars indicate that
-     * the email should be rejected
-     *
-     * Returns FALSE if the email should be acceptable. If the email should
-     * be rejected, the first filter that matches and has reject ticket set is
-     * returned.
-     */
-    function shouldReject() {
-        foreach ($this->getMatchingFilterList() as $filter) {
-            # Set reject if this filter indicates that the email should
-            # be blocked; however, don't unset $reject, because if it
-            # was set by another rule that did not set stopOnMatch(), we
-            # should still honor its configuration
-            if ($filter->rejectOnMatch()) return $filter;
-        }
-        return false;
-    }
     /**
      * Determine if any filters match the received email, and if so, apply
      * actions defined in those filters to the ticket-to-be-created.
+     *
+     * Throws:
+     * RejectedException if the email should not be acceptable. If the email
+     * should be rejected, the first filter that matches and has reject
+     * ticket set is returned.
      */
     function apply(&$ticket) {
         foreach ($this->getMatchingFilterList() as $filter) {
+            if ($filter->rejectOnMatch())
+                throw new RejectedException($filter);
             $filter->apply($ticket, $this->vars);
             if ($filter->stopOnMatch()) break;
         }
     }
 
-    /* static */ function getAllActive() {
+    function getAllActive() {
 
         $sql='SELECT id FROM '.FILTER_TABLE
             .' WHERE isactive=1 '
@@ -949,6 +938,19 @@ class TicketFilter {
     }
 }
 
+class RejectedException extends Exception {
+    var $filter;
+
+    function __construct(Filter $filter) {
+        parent::__construct('Ticket rejected by a filter');
+        $this->filter = $filter;
+    }
+
+    function getRejectingFilter() {
+        return $this->filter;
+    }
+}
+
 /**
  * Function: endsWith
  *
diff --git a/include/class.ticket.php b/include/class.ticket.php
index 643784722..ca3280235 100644
--- a/include/class.ticket.php
+++ b/include/class.ticket.php
@@ -2447,16 +2447,6 @@ class Ticket {
             }
         }
 
-        //Init ticket filters...
-        $ticket_filter = new TicketFilter($origin, $vars);
-        // Make sure email contents should not be rejected
-        if ($ticket_filter
-                && ($filter=$ticket_filter->shouldReject())) {
-            return $reject_ticket(
-                sprintf(_S('Ticket rejected (%s) by filter "%s"'),
-                    $vars['email'], $filter->getName()));
-        }
-
         $id=0;
         $fields=array();
         $fields['message']  = array('type'=>'*',     'required'=>1, 'error'=>__('Message content is required'));
@@ -2495,7 +2485,17 @@ class Ticket {
         if (!$errors) {
 
             # Perform ticket filter actions on the new ticket arguments
-            if ($ticket_filter) $ticket_filter->apply($vars);
+            try {
+                // Init ticket filters...
+                $ticket_filter = new TicketFilter($origin, $vars);
+                $ticket_filter->apply($vars);
+            }
+            catch (RejectedException $ex) {
+                return $reject_ticket(
+                    sprintf(_S('Ticket rejected (%s) by filter "%s"'),
+                    $vars['email'], $ex->getRejectingFilter()->getName())
+                );
+            }
 
             // Allow vars to be changed in ticket filter and applied to the user
             // account created or detected
diff --git a/include/class.user.php b/include/class.user.php
index ab6f3572b..99375d65a 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -318,7 +318,8 @@ class User extends UserModel {
             // Add in special `name` and `email` fields
             foreach (array('name', 'email') as $name) {
                 if ($f = $entry->getForm()->getField($name))
-                    $vars['field.'.$f->get('id')] = $this->getName();
+                    $vars['field.'.$f->get('id')] =
+                        $name == 'name' ? $this->getName() : $this->getEmail();
             }
         }
         return $vars;
-- 
GitLab