From f30462595e83f512cd5f29246a4542773595a224 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 13 Dec 2016 18:46:29 +0000 Subject: [PATCH] Optional Regex delimiters to truncate incoming emails (#5864). git-svn-id: http://svn.redmine.org/redmine/trunk@16065 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/settings_controller.rb | 31 +++++++++------- app/helpers/settings_helper.rb | 29 +++++++++++---- app/models/mail_handler.rb | 13 +++++-- app/models/setting.rb | 30 ++++++++++++++-- app/views/settings/_mail_handler.html.erb | 1 + app/views/settings/edit.html.erb | 2 ++ config/locales/en.yml | 2 ++ config/locales/fr.yml | 2 ++ config/settings.yml | 2 ++ .../mail_handler/ticket_reply_from_mail.eml | 35 +++++++++++++++++++ test/functional/settings_controller_test.rb | 29 +++++++++++++++ test/unit/mail_handler_test.rb | 19 ++++++++++ 12 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 test/fixtures/mail_handler/ticket_reply_from_mail.eml 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 @@

<%= 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) %> <%= l(:text_line_separated) %>

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 @@

<%= l(:label_settings) %>

+<%= 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: +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" +To: +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 > 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 .*, .* <.*> wrote:', + } + } + + assert_redirected_to '/settings' + assert_equal '1', Setting.mail_handler_enable_regex_delimiters + assert_equal 'On .*, .* at .*, .* <.*> 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 .*, .* <.*> 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 > 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 > 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'}) -- 2.39.5