Log info messages when MailHandler ignored a reply to a nonexistent issue, journal, or message (#31946).

Patch by Go MAEDA.


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

1 files changed, 34 insertions, 21 deletions

diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index 3510285f2..95a1e1742 100755
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -217,8 +217,12 @@ class MailHandler < ActionMailer::Base
 
   # Adds a note to an existing issue
   def receive_issue_reply(issue_id, from_journal=nil)
-    issue = Issue.find_by_id(issue_id)
-    return unless issue
+    issue = Issue.find_by(:id => issue_id)
+    if issue.nil?
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a nonexistent issue"
+      return nil
+    end
+
     # check permission
     unless handler_options[:no_permission_check]
       unless user.allowed_to?(:add_issue_notes, issue.project) ||
@@ -249,33 +253,42 @@ class MailHandler < ActionMailer::Base
 
   # Reply will be added to the issue
   def receive_journal_reply(journal_id)
-    journal = Journal.find_by_id(journal_id)
-    if journal && journal.journalized_type == 'Issue'
+    journal = Journal.find_by(:id => journal_id)
+    if journal.nil?
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a nonexistent journal"
+      return nil
+    end
+
+    if journal.journalized_type == 'Issue'
       receive_issue_reply(journal.journalized_id, journal)
+    else
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a journal whose journalized_type is not Issue"
+      return nil
     end
   end
 
   # Receives a reply to a forum message
   def receive_message_reply(message_id)
-    message = Message.find_by_id(message_id)
-    if message
-      message = message.root
+    message = Message.find_by(:id => message_id)&.root
+    if message.nil?
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a nonexistent message"
+      return nil
+    end
 
-      unless handler_options[:no_permission_check]
-        raise UnauthorizedAction, "not allowed to add messages to project [#{project.name}]" unless user.allowed_to?(:add_messages, message.project)
-      end
+    unless handler_options[:no_permission_check]
+      raise UnauthorizedAction, "not allowed to add messages to project [#{project.name}]" unless user.allowed_to?(:add_messages, message.project)
+    end
 
-      if !message.locked?
-        reply = Message.new(:subject => cleaned_up_subject.gsub(%r{^.*msg\d+\]}, '').strip,
-                            :content => cleaned_up_text_body)
-        reply.author = user
-        reply.board = message.board
-        message.children << reply
-        add_attachments(reply)
-        reply
-      else
-        logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a locked topic"
-      end
+    if !message.locked?
+      reply = Message.new(:subject => cleaned_up_subject.gsub(%r{^.*msg\d+\]}, '').strip,
+                          :content => cleaned_up_text_body)
+      reply.author = user
+      reply.board = message.board
+      message.children << reply
+      add_attachments(reply)
+      reply
+    else
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a locked topic"
     end
   end