From 8a44d286205814d1ce16116795f3cc225e0ec4a2 Mon Sep 17 00:00:00 2001
From: MAEDA Go <maeda@farend.jp>
Date: Sun, 25 Aug 2019 17:17:35 +0900
Subject: [PATCH] Output log entry when MailHandler ignored a reply to a
 nonexistent issue or message

---
 app/models/mail_handler.rb     | 45 +++++++++++++++++++---------------
 test/unit/mail_handler_test.rb | 18 ++++++++++++++
 2 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index f0f0f41b1..cb9e06013 100755
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -217,8 +217,11 @@ 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
+    unless (issue = Issue.find_by(:id => issue_id))
+      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) ||
@@ -231,7 +234,7 @@ class MailHandler < ActionMailer::Base
     handler_options[:issue] = {}
 
     journal = issue.init_journal(user)
-    if from_journal && from_journal.private_notes?
+    if from_journal&.private_notes?
       # If the received email was a reply to a private note, make the added note private
       issue.private_notes = true
     end
@@ -257,25 +260,27 @@ class MailHandler < ActionMailer::Base
 
   # 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
+    unless (message = Message.find_by(:id => message_id))
+      logger&.info "MailHandler: ignoring reply from [#{email.from.first}] to a nonexistent message"
+      return
+    end
 
-      unless handler_options[:no_permission_check]
-        raise UnauthorizedAction unless user.allowed_to?(:add_messages, message.project)
-      end
+    message = message.root
 
-      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
+    unless handler_options[:no_permission_check]
+      raise UnauthorizedAction unless user.allowed_to?(:add_messages, message.project)
+    end
+
+    if !message.locked?
+      reply = Message.new(:subject => cleaned_up_subject.gsub(/^.*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
 
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index 758f5f6e8..3ec48f1fe 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -985,6 +985,16 @@ class MailHandlerTest < ActiveSupport::TestCase
     end
   end
 
+  def test_reply_to_a_nonexistent_issue
+    Issue.find(2).destroy
+    assert_no_difference 'Issue.count' do
+      assert_no_difference 'Journal.count' do
+        journal = submit_email('ticket_reply_with_status.eml')
+        assert_nil journal
+      end
+    end
+  end
+
   def test_reply_to_a_message
     m = submit_email('message_reply.eml')
     assert m.is_a?(Message)
@@ -1015,6 +1025,14 @@ class MailHandlerTest < ActiveSupport::TestCase
     end
   end
 
+  def test_reply_to_a_nonexistent_topic
+    Message.find(2).destroy
+    assert_no_difference('Message.count') do
+      m = submit_email('message_reply_by_subject.eml')
+      assert_nil m
+    end
+  end
+
   def test_should_convert_tags_of_html_only_emails
     with_settings :text_formatting => 'textile' do
       issue = submit_email('ticket_html_only.eml', :issue => {:project => 'ecookbook'})
-- 
2.20.1 (Apple Git-117)