Optional Regex delimiters to truncate incoming emails (#5864).

git-svn-id: http://svn.redmine.org/redmine/trunk@16065 e93f8b46-1217-0410-a6f0-8f06a7374b81

12 files changed, 172 insertions, 23 deletions

diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb
index 6b7e51874..7ef7e9945 100644
--- a/app/controllers/settings_controller.rb
+++ b/app/controllers/settings_controller.rb
@@ -34,24 +34,29 @@ class SettingsController < ApplicationController
   def edit
     @notifiables = Redmine::Notifiable.all
     if request.post?
-      if Setting.set_all_from_params(params[:settings])
+      errors = Setting.set_all_from_params(params[:settings])
+      if errors.blank?
         flash[:notice] = l(:notice_successful_update)
+        redirect_to settings_path(:tab => params[:tab])
+        return
+      else
+        @setting_errors = errors
+        # render the edit form with error messages
       end
-      redirect_to settings_path(:tab => params[:tab])
-    else
-      @options = {}
-      user_format = User::USER_FORMATS.collect{|key, value| [key, value[:setting_order]]}.sort{|a, b| a[1] <=> b[1]}
-      @options[:user_format] = user_format.collect{|f| [User.current.name(f[0]), f[0].to_s]}
-      @deliveries = ActionMailer::Base.perform_deliveries
+    end
 
-      @guessed_host_and_path = request.host_with_port.dup
-      @guessed_host_and_path << ('/'+ Redmine::Utils.relative_url_root.gsub(%r{^\/}, '')) unless Redmine::Utils.relative_url_root.blank?
+    @options = {}
+    user_format = User::USER_FORMATS.collect{|key, value| [key, value[:setting_order]]}.sort{|a, b| a[1] <=> b[1]}
+    @options[:user_format] = user_format.collect{|f| [User.current.name(f[0]), f[0].to_s]}
+    @deliveries = ActionMailer::Base.perform_deliveries
 
-      @commit_update_keywords = Setting.commit_update_keywords.dup
-      @commit_update_keywords = [{}] unless @commit_update_keywords.is_a?(Array) && @commit_update_keywords.any?
+    @guessed_host_and_path = request.host_with_port.dup
+    @guessed_host_and_path << ('/'+ Redmine::Utils.relative_url_root.gsub(%r{^\/}, '')) unless Redmine::Utils.relative_url_root.blank?
 
-      Redmine::Themes.rescan
-    end
+    @commit_update_keywords = Setting.commit_update_keywords.dup
+    @commit_update_keywords = [{}] unless @commit_update_keywords.is_a?(Array) && @commit_update_keywords.any?
+
+    Redmine::Themes.rescan
   end
 
   def plugin
diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb
index 2c657f0df..1bba0f9fa 100644
--- a/app/helpers/settings_helper.rb
+++ b/app/helpers/settings_helper.rb
@@ -32,18 +32,35 @@ module SettingsHelper
             ]
   end
 
+  def render_settings_error(errors)
+    return if errors.blank?
+    s = ''.html_safe
+    errors.each do |name, message|
+      s << content_tag('li', content_tag('b', l("setting_#{name}")) + " " + message)
+    end
+    content_tag('div', content_tag('ul', s), :id => 'errorExplanation')
+  end
+
+  def setting_value(setting)
+    value = nil
+    if params[:settings]
+      value = params[:settings][setting]
+    end
+    value || Setting.send(setting)
+  end
+
   def setting_select(setting, choices, options={})
     if blank_text = options.delete(:blank)
       choices = [[blank_text.is_a?(Symbol) ? l(blank_text) : blank_text, '']] + choices
     end
     setting_label(setting, options).html_safe +
       select_tag("settings[#{setting}]",
-                 options_for_select(choices, Setting.send(setting).to_s),
+                 options_for_select(choices, setting_value(setting).to_s),
                  options).html_safe
   end
 
   def setting_multiselect(setting, choices, options={})
-    setting_values = Setting.send(setting)
+    setting_values = setting_value(setting)
     setting_values = [] unless setting_values.is_a?(Array)
 
     content_tag("label", l(options[:label] || "setting_#{setting}")) +
@@ -65,18 +82,18 @@ module SettingsHelper
 
   def setting_text_field(setting, options={})
     setting_label(setting, options).html_safe +
-      text_field_tag("settings[#{setting}]", Setting.send(setting), options).html_safe
+      text_field_tag("settings[#{setting}]", setting_value(setting), options).html_safe
   end
 
   def setting_text_area(setting, options={})
     setting_label(setting, options).html_safe +
-      text_area_tag("settings[#{setting}]", Setting.send(setting), options).html_safe
+      text_area_tag("settings[#{setting}]", setting_value(setting), options).html_safe
   end
 
   def setting_check_box(setting, options={})
     setting_label(setting, options).html_safe +
       hidden_field_tag("settings[#{setting}]", 0, :id => nil).html_safe +
-        check_box_tag("settings[#{setting}]", 1, Setting.send("#{setting}?"), options).html_safe
+        check_box_tag("settings[#{setting}]", 1, setting_value(setting).to_s != '0', options).html_safe
   end
 
   def setting_label(setting, options={})
@@ -97,7 +114,7 @@ module SettingsHelper
 
     tag = check_box_tag('settings[notified_events][]',
       notifiable.name,
-      Setting.notified_events.include?(notifiable.name),
+      setting_value('notified_events').include?(notifiable.name),
       :id => nil,
       :data => tag_data)
 
diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index 28841310e..402be4f51 100644
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -561,9 +561,18 @@ class MailHandler < ActionMailer::Base
 
   # Removes the email body of text after the truncation configurations.
   def cleanup_body(body)
-    delimiters = Setting.mail_handler_body_delimiters.to_s.split(/[\r\n]+/).reject(&:blank?).map {|s| Regexp.escape(s)}
+    delimiters = Setting.mail_handler_body_delimiters.to_s.split(/[\r\n]+/).reject(&:blank?)
+
+    if Setting.mail_handler_enable_regex_delimiters?
+      begin
+        delimiters = delimiters.map {|s| Regexp.new(s)}
+      rescue RegexpError => e
+        logger.error "MailHandler: invalid regexp delimiter found in mail_handler_body_delimiters setting (#{e.message})" if logger
+      end
+    end
+
     unless delimiters.empty?
-      regex = Regexp.new("^[> ]*(#{ delimiters.join('|') })\s*[\r\n].*", Regexp::MULTILINE)
+      regex = Regexp.new("^[> ]*(#{ Regexp.union(delimiters) })\s*[\r\n].*", Regexp::MULTILINE)
       body = body.gsub(regex, '')
     end
     body.strip
diff --git a/app/models/setting.rb b/app/models/setting.rb
index 934e6bf2a..321b1cd38 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -120,8 +120,12 @@ class Setting < ActiveRecord::Base
 
 	# Updates multiple settings from params and sends a security notification if needed
   def self.set_all_from_params(settings)
-    return false unless settings.is_a?(Hash)
+    return nil unless settings.is_a?(Hash)
     settings = settings.dup.symbolize_keys
+
+    errors = validate_all_from_params(settings)
+    return errors if errors.present?
+
     changes = []
     settings.each do |name, value|
       next unless available_settings[name.to_s]
@@ -134,7 +138,29 @@ class Setting < ActiveRecord::Base
     if changes.any?
       Mailer.security_settings_updated(changes)
     end
-    true
+    nil
+  end
+
+  def self.validate_all_from_params(settings)
+    messages = []
+
+    if settings.key?(:mail_handler_body_delimiters) || settings.key?(:mail_handler_enable_regex_delimiters)
+      regexp = Setting.mail_handler_enable_regex_delimiters?
+      if settings.key?(:mail_handler_enable_regex_delimiters)
+        regexp = settings[:mail_handler_enable_regex_delimiters].to_s != '0'
+      end
+      if regexp
+        settings[:mail_handler_body_delimiters].to_s.split(/[\r\n]+/).each do |delimiter|
+          begin
+            Regexp.new(delimiter)
+          rescue RegexpError => e
+            messages << [:mail_handler_body_delimiters, "#{l('activerecord.errors.messages.not_a_regexp')} (#{e.message})"]
+          end
+        end
+      end
+    end
+
+    messages
   end
 
   # Sets a setting value from params
diff --git a/app/views/settings/_mail_handler.html.erb b/app/views/settings/_mail_handler.html.erb
index f255b4adc..02dfe5264 100644
--- a/app/views/settings/_mail_handler.html.erb
+++ b/app/views/settings/_mail_handler.html.erb
@@ -3,6 +3,7 @@
 <div class="box tabular settings">
   <p>
     <%= setting_text_area :mail_handler_body_delimiters, :rows => 5 %>
+    <%= setting_check_box :mail_handler_enable_regex_delimiters, :label => false %> <%= l(:setting_mail_handler_enable_regex_delimiters) %>
     <em class="info"><%= l(:text_line_separated) %></em>
   </p>
   <p>
diff --git a/app/views/settings/edit.html.erb b/app/views/settings/edit.html.erb
index de64e3bb2..3db255e29 100644
--- a/app/views/settings/edit.html.erb
+++ b/app/views/settings/edit.html.erb
@@ -1,5 +1,7 @@
 <h2><%= l(:label_settings) %></h2>
 
+<%= render_settings_error @setting_errors %>
+
 <%= render_tabs administration_settings_tabs %>
 
 <% html_title(l(:label_settings), l(:label_administration)) -%>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index ba839aa78..669ea472f 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -130,6 +130,7 @@ en:
         circular_dependency: "This relation would create a circular dependency"
         cant_link_an_issue_with_a_descendant: "An issue cannot be linked to one of its subtasks"
         earlier_than_minimum_start_date: "cannot be earlier than %{date} because of preceding issues"
+        not_a_regexp: "is not a valid regular expression"
 
   actionview_instancetag_blank_option: Please select
 
@@ -403,6 +404,7 @@ en:
   setting_display_subprojects_issues: Display subprojects issues on main projects by default
   setting_enabled_scm: Enabled SCM
   setting_mail_handler_body_delimiters: "Truncate emails after one of these lines"
+  setting_mail_handler_enable_regex_delimiters: "Enable regular expressions"
   setting_mail_handler_api_enabled: Enable WS for incoming emails
   setting_mail_handler_api_key: Incoming email WS API key
   setting_sys_api_key: Repository management WS API key
diff --git a/config/locales/fr.yml b/config/locales/fr.yml
index abf39c8e6..a55c1883b 100644
--- a/config/locales/fr.yml
+++ b/config/locales/fr.yml
@@ -150,6 +150,7 @@ fr:
         circular_dependency: "Cette relation créerait une dépendance circulaire"
         cant_link_an_issue_with_a_descendant: "Une demande ne peut pas être liée à l'une de ses sous-tâches"
         earlier_than_minimum_start_date: "ne peut pas être antérieure au %{date} à cause des demandes qui précèdent"
+        not_a_regexp: "n'est pas une expression regulière valide"
 
   actionview_instancetag_blank_option: Choisir
 
@@ -415,6 +416,7 @@ fr:
   setting_display_subprojects_issues: Afficher par défaut les demandes des sous-projets sur les projets principaux
   setting_enabled_scm: SCM activés
   setting_mail_handler_body_delimiters: "Tronquer les emails après l'une de ces lignes"
+  setting_mail_handler_enable_regex_delimiters: "Utiliser les expressions regulières"
   setting_mail_handler_api_enabled: "Activer le WS pour la réception d'emails"
   setting_mail_handler_api_key: Clé de protection de l'API
   setting_sequential_project_identifiers: Générer des identifiants de projet séquentiels
diff --git a/config/settings.yml b/config/settings.yml
index 8dd8b3dc9..1411a0143 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -182,6 +182,8 @@ notified_events:
   - issue_updated
 mail_handler_body_delimiters:
   default: ''
+mail_handler_enable_regex_delimiters:
+  default: 0
 mail_handler_excluded_filenames:
   default: ''
 mail_handler_api_enabled:
diff --git a/test/fixtures/mail_handler/ticket_reply_from_mail.eml b/test/fixtures/mail_handler/ticket_reply_from_mail.eml
new file mode 100644
index 000000000..016b189f1
--- /dev/null
+++ b/test/fixtures/mail_handler/ticket_reply_from_mail.eml
@@ -0,0 +1,35 @@
+Return-Path: <jsmith@somenet.foo>
+Received: from osiris ([127.0.0.1])
+	by OSIRIS
+	with hMailServer; Wed, 12 Oct 2016 03:05:50 -0700
+Message-ID: <000501c8d452$a95cd7e0$0a00a8c0@osiris>
+From: "John Smith" <JSmith@somenet.foo>
+To: <redmine@somenet.foo>
+Subject: New ticket on a given project
+Date: Wed, 12 Oct 2016 13:05:38 +0300
+MIME-Version: 1.0
+Content-Type: text/plain;
+	format=flowed;
+	charset="iso-8859-1";
+	reply-type=original
+Content-Transfer-Encoding: 7bit
+X-Priority: 3
+X-MSMail-Priority: Normal
+X-Mailer: Microsoft Outlook Express 6.00.2900.2869
+X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2869
+
+Project: onlinestore
+Status: Resolved
+due date: 2010-12-31
+Start Date:2010-01-01
+Assigned to: John Smith
+fixed version: alpha
+estimated hours: 2.5
+remaining hours: 1
+done ratio: 30
+
+This paragraph is before delimiter
+
+On Wed, 11 Oct at 1:05 PM, Jon Smith <jsmith@somenet.foo<mailto:jsmith@somenet.foo>> wrote:
+
+This paragraph is after the delimiter
diff --git a/test/functional/settings_controller_test.rb b/test/functional/settings_controller_test.rb
index 38e569569..d7dacc0ee 100644
--- a/test/functional/settings_controller_test.rb
+++ b/test/functional/settings_controller_test.rb
@@ -254,4 +254,33 @@ class SettingsControllerTest < Redmine::ControllerTest
   ensure
     Redmine::Plugin.unregister(:foo)
   end
+
+  def test_post_mail_handler_delimiters_should_not_save_invalid_regex_delimiters
+    post :edit, :params => {
+      :settings => {
+        :mail_handler_enable_regex_delimiters => '1',
+        :mail_handler_body_delimiters  => 'Abc[',
+      }
+    }
+
+    assert_response :success
+    assert_equal '0', Setting.mail_handler_enable_regex_delimiters
+    assert_equal '', Setting.mail_handler_body_delimiters
+
+    assert_select_error /is not a valid regular expression/
+    assert_select 'textarea[name=?]', 'settings[mail_handler_body_delimiters]', :text => 'Abc['
+  end
+
+  def test_post_mail_handler_delimiters_should_save_valid_regex_delimiters
+    post :edit, :params => {
+      :settings => {
+        :mail_handler_enable_regex_delimiters => '1',
+        :mail_handler_body_delimiters  => 'On .*, .* at .*, .* <.*<mailto:.*>> wrote:',
+      }
+    }
+
+    assert_redirected_to '/settings'
+    assert_equal '1', Setting.mail_handler_enable_regex_delimiters
+    assert_equal 'On .*, .* at .*, .* <.*<mailto:.*>> wrote:', Setting.mail_handler_body_delimiters
+  end
 end
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index c1afdaebd..898ccc918 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -977,6 +977,25 @@ class MailHandlerTest < ActiveSupport::TestCase
     end
   end
 
+  test "truncate emails using a regex delimiter" do
+    delimiter = "On .*, .* at .*, .* <.*<mailto:.*>> wrote:"
+    with_settings :mail_handler_enable_regex_delimiters => '1', :mail_handler_body_delimiters => delimiter do
+      issue = submit_email('ticket_reply_from_mail.eml')
+      assert_issue_created(issue)
+      assert issue.description.include?('This paragraph is before delimiter')
+      assert !issue.description.include?('On Wed, 11 Oct at 1:05 PM, Jon Smith <jsmith@somenet.foo<mailto:jsmith@somenet.foo>> wrote:')
+      assert !issue.description.include?('This paragraph is after the delimiter')
+    end
+
+    with_settings :mail_handler_enable_regex_delimiters => '0', :mail_handler_body_delimiters => delimiter do
+      issue = submit_email('ticket_reply_from_mail.eml')
+      assert_issue_created(issue)
+      assert issue.description.include?('This paragraph is before delimiter')
+      assert issue.description.include?('On Wed, 11 Oct at 1:05 PM, Jon Smith <jsmith@somenet.foo<mailto:jsmith@somenet.foo>> wrote:')
+      assert issue.description.include?('This paragraph is after the delimiter')
+    end
+  end
+
   def test_attachments_that_match_mail_handler_excluded_filenames_should_be_ignored
     with_settings :mail_handler_excluded_filenames => '*.vcf, *.jpg' do
       issue = submit_email('ticket_with_attachment.eml', :issue => {:project => 'onlinestore'})