From 378d19c691bdab84f9f9ae694f83a2b00756f9d2 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Tue, 24 Oct 2017 16:58:03 +0200 Subject: [PATCH] Merged pull request #1483 - Working on issue #1439: Refactoring - moved out the regex matching code to better readability. Huge thanks to @muhammadn . --- app/models/channel/filter/database.rb | 32 +---- .../channel/filter/match/email_regex.rb | 27 ++++ app/models/channel/filter/monitoring_base.rb | 3 +- app/models/postmaster_filter.rb | 6 +- test/unit/email_regex_test.rb | 117 ++++++++++++++++++ 5 files changed, 148 insertions(+), 37 deletions(-) create mode 100644 app/models/channel/filter/match/email_regex.rb create mode 100644 test/unit/email_regex_test.rb diff --git a/app/models/channel/filter/database.rb b/app/models/channel/filter/database.rb index b83ace767..8161da2ee 100644 --- a/app/models/channel/filter/database.rb +++ b/app/models/channel/filter/database.rb @@ -18,12 +18,12 @@ module Channel::Filter::Database match_rule = meta['value'] min_one_rule_exists = true if meta[:operator] == 'contains not' - if value.present? && match(value, match_rule, false) + if value.present? && Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule) all_matches_ok = false Rails.logger.info " matching #{key.downcase}:'#{value}' on #{match_rule}, but shoud not" end elsif meta[:operator] == 'contains' - if value.blank? || !match(value, match_rule, true) + if value.blank? || !Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule) all_matches_ok = false Rails.logger.info " not matching #{key.downcase}:'#{value}' on #{match_rule}, but should" end @@ -51,32 +51,4 @@ module Channel::Filter::Database end - def self.match(value, match_rule, _should_match, check_mode = false) - - regexp = false - if match_rule =~ /^(regex:)(.+?)$/ - regexp = true - match_rule = $2 - end - - value ||= '' - - if regexp == false - match_rule_quoted = Regexp.quote(match_rule).gsub(/\\\*/, '.*') - return true if value =~ /#{match_rule_quoted}/i - return false - end - - begin - return true if value =~ /#{match_rule}/i - return false - rescue => e - message = "Can't use regex '#{match_rule}' on '#{value}': #{e.message}" - Rails.logger.error message - raise message if check_mode == true - end - - false - end - end diff --git a/app/models/channel/filter/match/email_regex.rb b/app/models/channel/filter/match/email_regex.rb new file mode 100644 index 000000000..185f48be0 --- /dev/null +++ b/app/models/channel/filter/match/email_regex.rb @@ -0,0 +1,27 @@ +module Channel::Filter::Match::EmailRegex + + def self.match(value:, match_rule:, check_mode: false) + regexp = false + if match_rule =~ /^(regex:)(.+?)$/ + regexp = true + match_rule = $2 + end + + if regexp == false + match_rule_quoted = Regexp.quote(match_rule).gsub(/\\\*/, '.*') + return true if value =~ /#{match_rule_quoted}/i + return false + end + + begin + return true if value =~ /#{match_rule}/i + return false + rescue => e + message = "Can't use regex '#{match_rule}' on '#{value}': #{e.message}" + Rails.logger.error message + raise message if check_mode == true + end + + false + end +end diff --git a/app/models/channel/filter/monitoring_base.rb b/app/models/channel/filter/monitoring_base.rb index db73a65e2..97e7651a1 100644 --- a/app/models/channel/filter/monitoring_base.rb +++ b/app/models/channel/filter/monitoring_base.rb @@ -1,7 +1,6 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Channel::Filter::MonitoringBase - # according # Icinga @@ -27,7 +26,7 @@ class Channel::Filter::MonitoringBase return if !session_user_id # check if sender is monitoring - return if !Channel::Filter::Database.match(mail[:from], sender, true, true) + return if !Channel::Filter::Match::EmailRegex.match(value: mail[:from], match_rule: sender, check_mode: true) # get mail attibutes like host and state result = {} diff --git a/app/models/postmaster_filter.rb b/app/models/postmaster_filter.rb index e0d2ac04d..9a5f72361 100644 --- a/app/models/postmaster_filter.rb +++ b/app/models/postmaster_filter.rb @@ -14,11 +14,7 @@ class PostmasterFilter < ApplicationModel raise Exceptions::UnprocessableEntity, 'operator invalid, ony "contains" and "contains not" is supported' if meta['operator'].blank? || meta['operator'] !~ /^(contains|contains not)$/ raise Exceptions::UnprocessableEntity, 'value invalid/empty' if meta['value'].blank? begin - if meta['operator'] == 'contains not' - Channel::Filter::Database.match('test content', meta['value'], false, true) - else - Channel::Filter::Database.match('test content', meta['value'], true, true) - end + Channel::Filter::Match::EmailRegex.match(value: 'test content', match_rule: meta['value'], check_mode: true) rescue => e raise Exceptions::UnprocessableEntity, e.message end diff --git a/test/unit/email_regex_test.rb b/test/unit/email_regex_test.rb new file mode 100644 index 000000000..0052fa06f --- /dev/null +++ b/test/unit/email_regex_test.rb @@ -0,0 +1,117 @@ +# encoding: utf-8 +require 'test_helper' + +class EmailRegexTest < ActiveSupport::TestCase + + test 'should be able to detect valid/invalid the regex filter' do + # check with exact email, check_mode = true + sender = 'foobar@foo.bar' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(true, regex) + + # check with exact email + sender = 'foobar@foo.bar' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(true, regex) + + # check with regex: filter check_mode = true + sender = 'regex:foobar@.*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(true, regex) + + # check with regex: filter + sender = 'regex:foobar@.*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(true, regex) + + # check regex with regex: filter + sender = 'regex:??' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(false, regex) + + # check regex with raise error (check_mode = true) + assert_raises("Can't use regex '??' on 'foobar@foo.bar'") do + sender = 'regex:??' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + end + + sender = 'regex:[]' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(false, regex) + + # check regex with raise error, (check_mode = true) + assert_raises("Can't use regex '[]' on 'foobar@foo.bar'") do + sender = 'regex:[]' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + end + + # check regex with empty field + sender = '{}' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(false, regex) + + # check regex with empty field and raise error (check_mode = true) + sender = '{}' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(false, regex) + + # check regex with empty field + sender = '' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(true, regex) + + # check regex with empty field + sender = '' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(true, regex) + + # check regex with regex: wildcard + sender = 'regex:*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(false, regex) + + # check regex with regex: wildcard and raise error (check_mode = true) + assert_raises("Can't use regex '*' on 'foobar@foo.bar'") do + sender = 'regex:*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + end + + # check email with wildcard + sender = '*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(true, regex) + + # check email with wildcard (check_mode = true) + sender = '*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(true, regex) + + # check email with a different sender + sender = 'regex:nagios@.*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: false) + assert_equal(false, regex) + + # check email with a different sender with checkmode = true + sender = 'regex:nagios@.*' + from = 'foobar@foo.bar' + regex = Channel::Filter::Match::EmailRegex.match(value: from, match_rule: sender, check_mode: true) + assert_equal(false, regex) + end +end