From 7af0009321bbc7d910066cb317eb3245c5f792e2 Mon Sep 17 00:00:00 2001
From: Sabina Talipova <87288324+sabina-talipova@users.noreply.github.com>
Date: Wed, 17 May 2023 15:54:29 +1200
Subject: [PATCH] FIX Passing array to setReplyTo method instead of string
 (#1208)

---
 code/Control/UserDefinedFormController.php    | 31 +++++++++++++----
 .../Control/UserDefinedFormControllerTest.php | 33 +++++++++++++++++++
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/code/Control/UserDefinedFormController.php b/code/Control/UserDefinedFormController.php
index adbba86..deffc32 100644
--- a/code/Control/UserDefinedFormController.php
+++ b/code/Control/UserDefinedFormController.php
@@ -433,15 +433,17 @@ JS
                     $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailFromField()->Name);
 
                     if ($submittedFormField && $submittedFormField->Value && is_string($submittedFormField->Value)) {
-                        $email->setReplyTo(explode(',', $submittedFormField->Value ?? ''));
+                        $emailSendTo = $this->validEmailsToArray($submittedFormField->Value);
+                        $email->addReplyTo(...$emailSendTo);
                     }
                 } elseif ($recipient->EmailReplyTo) {
-                    $email->setReplyTo(explode(',', $recipient->EmailReplyTo ?? ''));
+                    $emailReplyTo = $this->validEmailsToArray($recipient->EmailReplyTo);
+                    $email->addReplyTo(...$emailReplyTo);
                 }
 
                 // check for a specified from; otherwise fall back to server defaults
                 if ($recipient->EmailFrom) {
-                    $email->setFrom(explode(',', $recipient->EmailFrom ?? ''));
+                    $email->setFrom($this->validEmailsToArray($recipient->EmailFrom));
                 }
 
                 // check to see if they are a dynamic reciever eg based on a dropdown field a user selected
@@ -452,12 +454,12 @@ JS
                         $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name);
 
                         if ($submittedFormField && is_string($submittedFormField->Value)) {
-                            $email->setTo(explode(',', $submittedFormField->Value ?? ''));
+                            $email->setTo($this->validEmailsToArray($submittedFormField->Value));
                         } else {
-                            $email->setTo(explode(',', $recipient->EmailAddress ?? ''));
+                            $email->setTo($this->validEmailsToArray($recipient->EmailAddress));
                         }
                     } else {
-                        $email->setTo(explode(',', $recipient->EmailAddress ?? ''));
+                        $email->setTo($this->validEmailsToArray($recipient->EmailAddress));
                     }
                 } catch (Swift_RfcComplianceException $e) {
                     // The sending address is empty and/or invalid. Log and skip sending.
@@ -672,4 +674,21 @@ EOS;
 
         return $result;
     }
+
+    /**
+     * Check validity of email and return array of valid emails
+     */
+    private function validEmailsToArray(?string $emails): array
+    {
+        $emailsArray = [];
+        $emails = explode(',', $emails ?? '');
+        foreach ($emails as $email) {
+            $email = trim($email);
+            if (Email::is_valid_address($email)) {
+                $emailsArray[] = $email;
+            }
+        }
+
+        return $emailsArray;
+    }
 }
diff --git a/tests/php/Control/UserDefinedFormControllerTest.php b/tests/php/Control/UserDefinedFormControllerTest.php
index 4530cb2..24d4e79 100644
--- a/tests/php/Control/UserDefinedFormControllerTest.php
+++ b/tests/php/Control/UserDefinedFormControllerTest.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\UserForms\Tests\Control;
 
+use ReflectionClass;
 use SilverStripe\Assets\Dev\TestAssetStore;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Folder;
@@ -580,4 +581,36 @@ class UserDefinedFormControllerTest extends FunctionalTest
         $controller = new SizeStringTestableController(); // extends UserDefinedFormController
         $controller->convertSizeStringToBytes($input);
     }
+
+    public function provideValidEmailsToArray()
+    {
+        return [
+            [[], [null]],
+            [[], [' , , ']],
+            [[], ['broken.email, broken@.email, broken2.@email']],
+            [
+                ['broken@email', 'correctemail@email.com'],
+                [', broken@email, email@-email.com,correctemail@email.com,']
+            ],
+            [
+                ['correctemail1@email.com', 'correctemail2@email.com', 'correctemail3@email.com'],
+                ['correctemail1@email.com, correctemail2@email.com, correctemail3@email.com']
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider provideValidEmailsToArray
+     * Test that provided email is valid
+     */
+    public function testValidEmailsToArray(array $expectedOutput, array $input)
+    {
+        $class = new ReflectionClass(UserDefinedFormController::class);
+        $method = $class->getMethod('validEmailsToArray');
+        $method->setAccessible(true);
+
+        $controller = new UserDefinedFormController();
+
+        $this->assertEquals($expectedOutput, $method->invokeArgs($controller, $input));
+    }
 }