Remove quotation marks from email addresses (fixes #2154)

app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee

@@ -30,11 +30,12 @@ class EmailReply extends App.Controller
 
       # remove system addresses
       localAddresses = App.EmailAddress.all()
-      forgeinRecipients = []
+      foreignRecipients = []
       recipientUsed = {}
       for recipient in recipients
         if !_.isEmpty(recipient.address)
           localRecipientAddress = recipient.address.toString().toLowerCase()
+
           if !recipientUsed[localRecipientAddress]
             recipientUsed[localRecipientAddress] = true
             localAddress = false
@@ -43,10 +44,10 @@ class EmailReply extends App.Controller
                 recipientUsed[localRecipientAddress] = true
                 localAddress = true
             if !localAddress
-              forgeinRecipients.push recipient
+              foreignRecipients.push recipient
 
       # check if reply all is neede
-      if forgeinRecipients.length > 1
+      if foreignRecipients.length > 1
         actions.push {
           name: 'reply all'
           type: 'emailReplyAll'

app/assets/javascripts/app/lib/app_post/utils.coffee

@@ -1034,32 +1034,20 @@ class App.Utils
       # filter for uniq recipients
       recipientAddresses = {}
       addAddresses = (addressLine, line) ->
-        lineNew = ''
         recipients = App.Utils.parseAddressListLocal(addressLine)
 
-        if !_.isEmpty(recipients)
+        recipients = recipients.map((r) -> r.toString().toLowerCase())
-          for recipient in recipients
+        recipients = _.reject(recipients, (r) -> _.isEmpty(r))
-            if !_.isEmpty(recipient)
+        recipients = _.reject(recipients, (r) -> isLocalAddress(r))
-              localRecipientAddress = recipient.toString().toLowerCase()
+        recipients = _.reject(recipients, (r) -> recipientAddresses[r])
+        recipients = _.each(recipients, (r) -> recipientAddresses[r] = true)
 
-              # check if address is not local
+        recipients.push(line) if !_.isEmpty(line)
-              if !isLocalAddress(localRecipientAddress)
 
-                # filter for uniq recipients
+        # see https://github.com/zammad/zammad/issues/2154
-                if !recipientAddresses[localRecipientAddress]
+        recipients = recipients.map((a) -> a.replace(/'(\S+@\S+\.\S+)'/, '$1'))
-                  recipientAddresses[localRecipientAddress] = true
 
-                  # add recipient
+        recipients.join(', ')
-                  if lineNew
-                    lineNew = lineNew + ', '
-                  lineNew = lineNew + localRecipientAddress
-
-        lineNew
-        if !_.isEmpty(line)
-          if !_.isEmpty(lineNew)
-            lineNew += ', '
-          lineNew += line
-        lineNew
 
       if articleNew.to
         articleNew.to = addAddresses(articleNew.to)

app/models/channel/email_parser.rb

@@ -491,14 +491,13 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
     # imported_fields = mail.header.fields.map { |f| [f.name.downcase, f.to_utf8] }.to_h
     raw_fields = mail.header.fields.map { |f| ["raw-#{f.name.downcase}", f] }.to_h
     custom_fields = {}.tap do |h|
-      validated_recipients = imported_fields.slice(*RECIPIENT_FIELDS)
+      h.replace(imported_fields.slice(*RECIPIENT_FIELDS)
-                                            .transform_values { |v| v.match?(EMAIL_REGEX) ? v : '' }
+                               .transform_values { |v| v.match?(EMAIL_REGEX) ? v : '' })
-      h.merge!(validated_recipients)
 
+      h['x-any-recipient'] = h.values.select(&:present?).join(', ')
       h['date']            = Time.zone.parse(mail.date.to_s) || imported_fields['date']
       h['message_id']      = imported_fields['message-id']
       h['subject']         = imported_fields['subject']&.sub(/^=\?us-ascii\?Q\?(.+)\?=$/, '\1')
-      h['x-any-recipient'] = validated_recipients.values.select(&:present?).join(', ')
     end
 
     [imported_fields, raw_fields, custom_fields].reduce({}.with_indifferent_access, &:merge)

app/models/channel/filter/identify_sender.rb

@@ -196,10 +196,10 @@ module Channel::Filter::IdentifySender
   end
 
   def self.cleanup_email(string)
-    string = string.downcase
+    string.downcase
-    string.strip!
+          .strip
-    string.delete!('"')
+          .delete('"')
-    string
+          .sub(/\A'(.*)'\z/, '\1')
   end
 
 end

public/assets/tests/html_utils.js

@@ -2648,6 +2648,42 @@ test('check getRecipientArticle format', function() {
   verify = App.Utils.getRecipientArticle(ticket, article, agent, article.type, email_addresses, false)
   deepEqual(verify, result)
 
+  customer = {
+    login: 'login',
+    firstname: 'firstname',
+    lastname: 'lastname',
+    email: "'customer@example.com'",
+  }
+  agent = {
+    login: 'login',
+    firstname: 'firstname',
+    lastname: 'lastname',
+    email: 'agent@example.com',
+  }
+  ticket = {
+    customer: customer,
+  }
+  article = {
+    message_id: 'message_id21',
+    created_by: agent,
+    type: {
+      name: 'email',
+    },
+    sender: {
+      name: 'Agent',
+    },
+    from: customer.email,
+    to: 'agent@example.com',
+  }
+  result = {
+    to:          'customer@example.com',
+    cc:          '',
+    body:        '',
+    in_reply_to: 'message_id21',
+  }
+  verify = App.Utils.getRecipientArticle(ticket, article, article.created_by, article.type)
+  deepEqual(verify, result)
+
 });
 
 test("contentTypeCleanup", function() {

spec/models/channel/email_parser_spec.rb

@@ -1,14 +1,16 @@
 require 'rails_helper'
 
 RSpec.describe Channel::EmailParser, type: :model do
-  let(:ticket) { create(:ticket) }
   let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail001.box') }
-  let(:raw_mail) { File.read(mail_file).sub(/(?<=^Subject: ).*$/, test_string) }
+  let(:raw_mail) { File.read(mail_file) }
-  let(:test_string) do
-    Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number
-  end
 
   describe '#process' do
+    let(:raw_mail) { File.read(mail_file).sub(/(?<=^Subject: ).*$/, test_string) }
+    let(:test_string) do
+      Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number
+    end
+    let(:ticket) { create(:ticket) }
+
     context 'when email subject contains ticket reference' do
       it 'adds message to ticket' do
         expect { described_class.new.process({}, raw_mail) }

test/unit/email_process_test.rb

@@ -3033,6 +3033,40 @@ Content-Type: text/html; charset=us-ascii; format=flowed
           },
         },
       },
+      {
+        data: <<~RAW_MAIL.chomp,
+          From: me@example.com
+          To: Bob Smith <'customer_outlook_recipient_not_in_address_book@example.com'>
+          Subject: some subject for outlook recipient issue
+          Content-Type: text/html; charset=us-ascii;
+
+          test
+          RAW_MAIL
+        success: true,
+        result: {
+          0 => {
+            priority: '2 normal',
+            title: 'some subject for outlook recipient issue',
+          },
+          1 => {
+            content_type: 'text/html',
+            body: 'test',
+            sender: 'Customer',
+            type: 'email',
+            internal: false,
+          },
+        },
+        verify: {
+          users: [
+            {
+              firstname: 'Bob',
+              lastname: 'Smith',
+              fullname: 'Bob Smith',
+              email: 'customer_outlook_recipient_not_in_address_book@example.com',
+            },
+          ],
+        },
+      },
       {
         data: File.read(Rails.root.join('test', 'data', 'mail', 'mail067.box')),
         success: true,