From 1f237546eaae26a633e825d1010d93dfcc3742ce Mon Sep 17 00:00:00 2001
From: Mark Vulfson <mvulfson@netflix.com>
Date: Fri, 10 May 2019 09:31:58 -0700
Subject: [PATCH] fix(email): fix email parser to not fail on multiple
 addresses (#545)

If a pipeline specified two email addressed in the notification that email notifcation
will not be sent.
This change moves the email address splitting/parsing logic to the least common denominator -
the `send` function since it can be called either by handling an email event directly or
indirectly via pipeline/stage events

Also, adding ability to split on semicolons in email addresses
---
 .../email/EmailNotificationService.groovy     | 33 +++++++++----------
 .../email/EmailNotificationServiceSpec.groovy |  2 +-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy
index 20f2a587d..02bd6e4d0 100644
--- a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy
+++ b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy
@@ -55,25 +55,11 @@ class EmailNotificationService implements NotificationService {
 
   @Override
   EchoResponse.Void handle(Notification notification) {
-    def expando = { String addresses ->
-      // could be comma and/or space separated
-      addresses = addresses.replaceAll(",", " ")
-      return addresses.split(" ").findAll { !it.isEmpty() }
-    }
-
-    def to = notification.to?.collect {
-      expando(it)
-    }?.flatten() as String[]
-
-    def cc = notification.cc?.collect {
-      expando(it)
-    }?.flatten() as String[]
-
     def subject = notificationTemplateEngine.build(notification, NotificationTemplateEngine.Type.SUBJECT)
     def body = notificationTemplateEngine.build(notification, NotificationTemplateEngine.Type.BODY)
 
     try {
-      send(to, cc, subject, body)
+      send(notification.to?.toArray(new String[0]), notification.cc?.toArray(new String[0]), subject, body)
     } catch (AddressException e) {
       throw new InvalidRequestException(e.message)
     }
@@ -85,12 +71,25 @@ class EmailNotificationService implements NotificationService {
     MimeMessage message = javaMailSender.createMimeMessage()
     MimeMessageHelper helper = new MimeMessageHelper(message, false, "utf-8")
 
+    def splitAddresses = { String addresses ->
+      // Split on space, comma, or semicolon
+      return addresses.split("[ ,;]").findAll { !it.isEmpty() }
+    }
+
     if (to) {
-      helper.setTo(to)
+      def toParsed = to.collect {
+        splitAddresses(it)
+      }?.flatten() as String[]
+
+      helper.setTo(toParsed)
     }
 
     if (cc) {
-      helper.setCc(cc)
+      def ccParsed = cc.collect {
+        splitAddresses(it)
+      }?.flatten() as String[]
+
+      helper.setCc(ccParsed)
     }
 
     if (to || cc) {
diff --git a/echo-notifications/src/test/groovy/com/netflix/spinnaker/echo/email/EmailNotificationServiceSpec.groovy b/echo-notifications/src/test/groovy/com/netflix/spinnaker/echo/email/EmailNotificationServiceSpec.groovy
index 5389c2214..63aabf345 100644
--- a/echo-notifications/src/test/groovy/com/netflix/spinnaker/echo/email/EmailNotificationServiceSpec.groovy
+++ b/echo-notifications/src/test/groovy/com/netflix/spinnaker/echo/email/EmailNotificationServiceSpec.groovy
@@ -96,6 +96,6 @@ class EmailNotificationServiceSpec extends Specification {
     ['receiver@localhost']                    | ['some-addr@localhost']                            || ['receiver@localhost']                      || ['some-addr@localhost']
     ['receiver@localhost another@localhost']  | ['some-addr@localhost some-other-addr@localhost']  || ['receiver@localhost', 'another@localhost'] || ['some-addr@localhost', 'some-other-addr@localhost']
     ['receiver@localhost,another@localhost']  | ['some-addr@localhost,some-other-addr@localhost']  || ['receiver@localhost', 'another@localhost'] || ['some-addr@localhost', 'some-other-addr@localhost']
-    ['receiver@localhost, another@localhost'] | ['some-addr@localhost, some-other-addr@localhost'] || ['receiver@localhost', 'another@localhost'] || ['some-addr@localhost', 'some-other-addr@localhost']
+    ['receiver@localhost; another@localhost'] | ['some-addr@localhost, some-other-addr@localhost'] || ['receiver@localhost', 'another@localhost'] || ['some-addr@localhost', 'some-other-addr@localhost']
   }
 }