From e31d2751cfbd2b958a89843b6ead3511c677635c Mon Sep 17 00:00:00 2001
From: Martin Edenhofer <me@edenhofer.de>
Date: Mon, 24 Sep 2018 20:16:28 +0200
Subject: [PATCH] Fixed issue #2254 - Unable to process spam email `"ERROR:
 #<Exceptions::UnprocessableEntity: Invalid email>"`.

---
 app/models/channel/filter/identify_sender.rb |  4 +++-
 spec/models/channel/email_parser_spec.rb     | 20 +++++++++++++++++++
 test/data/mail/mail076.box                   |  7 +++++++
 test/data/mail/mail076.yml                   |  9 +++++++++
 test/unit/email_process_test.rb              | 21 ++++++++++++++++++++
 5 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 test/data/mail/mail076.box
 create mode 100644 test/data/mail/mail076.yml

diff --git a/app/models/channel/filter/identify_sender.rb b/app/models/channel/filter/identify_sender.rb
index 69b4bf9e6..ddc72bedf 100644
--- a/app/models/channel/filter/identify_sender.rb
+++ b/app/models/channel/filter/identify_sender.rb
@@ -156,7 +156,7 @@ module Channel::Filter::IdentifySender
   end
 
   def self.populate_attributes!(attrs, **extras)
-    if attrs[:email].match?(/\S\s+\S/)
+    if attrs[:email].match?(/\S\s+\S/) || attrs[:email].match?(/^<|>$/)
       attrs[:preferences] = { mail_delivery_failed: true,
                               mail_delivery_failed_reason: 'invalid email',
                               mail_delivery_failed_data: Time.zone.now }
@@ -190,6 +190,8 @@ module Channel::Filter::IdentifySender
     string.downcase
           .strip
           .delete('"')
+          .delete(' ')             # see https://github.com/zammad/zammad/issues/2254
+          .sub(/^<|>$/, '')        # see https://github.com/zammad/zammad/issues/2254
           .sub(/\A'(.*)'\z/, '\1') # see https://github.com/zammad/zammad/issues/2154
           .gsub(/\s/, '')          # see https://github.com/zammad/zammad/issues/2198
   end
diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb
index 2c2609afe..512efc858 100644
--- a/spec/models/channel/email_parser_spec.rb
+++ b/spec/models/channel/email_parser_spec.rb
@@ -71,6 +71,26 @@ RSpec.describe Channel::EmailParser, type: :model do
       end
     end
 
+    # see https://github.com/zammad/zammad/issues/2254
+    context 'when sender address contains > (#2254)' do
+      let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail076.box') }
+      let(:sender_email) { 'millionslotteryspaintransfer@example.com' }
+
+      it 'removes them before creating a new user' do
+        expect { described_class.new.process({}, raw_mail) }
+          .to change { User.where(email: sender_email).count }.to(1)
+      end
+
+      it 'marks new user email as invalid' do
+        described_class.new.process({}, raw_mail)
+
+        expect(User.find_by(email: sender_email).preferences)
+          .to include('mail_delivery_failed' => true)
+          .and include('mail_delivery_failed_reason' => 'invalid email')
+          .and include('mail_delivery_failed_data' => a_kind_of(ActiveSupport::TimeWithZone))
+      end
+    end
+
     # see https://github.com/zammad/zammad/issues/2224
     context 'when header specifies Windows-1258 charset (#2224)' do
       let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail072.box') }
diff --git a/test/data/mail/mail076.box b/test/data/mail/mail076.box
new file mode 100644
index 000000000..7de6ec465
--- /dev/null
+++ b/test/data/mail/mail076.box
@@ -0,0 +1,7 @@
+From: "Millions Lottery Spain transfer"@example.com>
+To: info@example.de
+Subject: [SPAM] (19.9) Your Winning Notification!!!
+Date: 24 Sep 2018 10:22:54 -0700
+Message-ID: <20180924102254.B52A3EAFDF0F87BA@example.com>
+
+test spam
\ No newline at end of file
diff --git a/test/data/mail/mail076.yml b/test/data/mail/mail076.yml
new file mode 100644
index 000000000..183382913
--- /dev/null
+++ b/test/data/mail/mail076.yml
@@ -0,0 +1,9 @@
+--- !ruby/hash:ActiveSupport::HashWithIndifferentAccess
+from: '"Millions Lottery Spain transfer"@example.com>'
+from_email: '"Millions Lottery Spain transfer"@example.com>'
+from_display_name: Millions Lottery Spain transfer@example.com>
+to: info@example.de
+subject: "[SPAM] (19.9) Your Winning Notification!!!"
+body: test spam
+content_type: text/plain
+attachments: []
diff --git a/test/unit/email_process_test.rb b/test/unit/email_process_test.rb
index 7c3f952a3..9d7f99f92 100644
--- a/test/unit/email_process_test.rb
+++ b/test/unit/email_process_test.rb
@@ -3115,6 +3115,27 @@ Content-Type: text/html; charset=us-ascii; format=flowed
           ],
         },
       },
+      { # See https://github.com/zammad/zammad/issues/2254
+        data: File.read(Rails.root.join('test', 'data', 'mail', 'mail076.box')),
+        success: true,
+        result: {
+          1 => {
+            from: '"Millions Lottery Spain transfer"@example.com>',
+            sender: 'Customer',
+            type: 'email',
+          },
+        },
+        verify: {
+          users: [
+            {
+              firstname: 'Millions',
+              lastname: 'Lottery Spain transfer@example.com>',
+              fullname: 'Millions Lottery Spain transfer@example.com>',
+              email: 'millionslotteryspaintransfer@example.com',
+            },
+          ],
+        },
+      },
     ]
     assert_process(files)
   end