From ca07a85157b9a2fca20309210619e6736447c9d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuli=20Sillanp=C3=A4=C3=A4?=
<samuli.sillanpaa@helsinki.fi>
Date: Fri, 23 Aug 2019 14:08:11 +0300
Subject: [PATCH] Prevent multiple reply-to addresses in Mailer. (#1406)
- Includes tests.
---
config/vufind/config.ini | 2 +
.../VuFind/Controller/FeedbackController.php | 3 --
module/VuFind/src/VuFind/Mailer/Mailer.php | 6 ++-
.../src/VuFindTest/Mailer/MailerTest.php | 46 +++++++++++++++++++
4 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/config/vufind/config.ini b/config/vufind/config.ini
index 5bc3467b7ba..b1d5dc0da2f 100644
--- a/config/vufind/config.ini
+++ b/config/vufind/config.ini
@@ -528,6 +528,8 @@ disable_from = false
; From field override. Setting this allows keeping the "from" field in email forms
; but will only use it as a reply-to address. The address defined here is used as the
; actual "from" address.
+; Note: If a feature explicitly sets a different reply-to address (for example,
+; Feedback forms), the original from address will NOT override that reply-to value.
;override_from = "no-reply@myuniversity.edu"
; Being a special case of mail message, sending record results via SMS ("Text this")
diff --git a/module/VuFind/src/VuFind/Controller/FeedbackController.php b/module/VuFind/src/VuFind/Controller/FeedbackController.php
index 41cd71c232e..7ff54f2412c 100644
--- a/module/VuFind/src/VuFind/Controller/FeedbackController.php
+++ b/module/VuFind/src/VuFind/Controller/FeedbackController.php
@@ -149,9 +149,6 @@ class FeedbackController extends AbstractBase
) {
try {
$mailer = $this->serviceLocator->get(\VuFind\Mailer\Mailer::class);
- if ($replyToEmail) {
- $mailer->setFromAddressOverride('');
- }
$mailer->send(
new Address($recipientEmail, $recipientName),
new Address($senderEmail, $senderName),
diff --git a/module/VuFind/src/VuFind/Mailer/Mailer.php b/module/VuFind/src/VuFind/Mailer/Mailer.php
index 193cb5823ca..ad096ba9a54 100644
--- a/module/VuFind/src/VuFind/Mailer/Mailer.php
+++ b/module/VuFind/src/VuFind/Mailer/Mailer.php
@@ -186,7 +186,11 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
if (!empty($this->fromAddressOverride)
&& $this->fromAddressOverride != $fromEmail
) {
- $replyTo->add($fromEmail);
+ // Add the original from address as the reply-to address unless
+ // a reply-to address has been specified
+ if (count($replyTo) === 0) {
+ $replyTo->add($fromEmail);
+ }
if (!($from instanceof Address)) {
$from = new Address($from);
}
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
index fc07636a14d..4f6d852489d 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
@@ -148,6 +148,52 @@ class MailerTest extends \VuFindTest\Unit\TestCase
$mailer->send('to@example.com', $address, 'subject', 'body');
}
+ /**
+ * Test sending an email using an explicitly set reply-to address.
+ *
+ * @return void
+ */
+ public function testSendWithReplyTo()
+ {
+ $callback = function ($message) {
+ $fromString = $message->getFrom()->current()->toString();
+ return '<to@example.com>' == $message->getTo()->current()->toString()
+ && '<reply-to@example.com>' == $message->getReplyTo()->current()->toString()
+ && '<me@example.com>' == $fromString
+ && 'body' == $message->getBody()
+ && 'subject' == $message->getSubject();
+ };
+ $transport = $this->createMock(\Zend\Mail\Transport\TransportInterface::class);
+ $transport->expects($this->once())->method('send')->with($this->callback($callback));
+ $address = new Address('me@example.com');
+ $mailer = new Mailer($transport);
+ $mailer->send('to@example.com', $address, 'subject', 'body', null, 'reply-to@example.com');
+ }
+
+ /**
+ * Test sending an email using a from address override
+ * and an explicitly set reply-to address.
+ *
+ * @return void
+ */
+ public function testSendWithFromOverrideAndReplyTo()
+ {
+ $callback = function ($message) {
+ $fromString = $message->getFrom()->current()->toString();
+ return '<to@example.com>' == $message->getTo()->current()->toString()
+ && '<reply-to@example.com>' == $message->getReplyTo()->current()->toString()
+ && 'me <no-reply@example.com>' == $fromString
+ && 'body' == $message->getBody()
+ && 'subject' == $message->getSubject();
+ };
+ $transport = $this->createMock(\Zend\Mail\Transport\TransportInterface::class);
+ $transport->expects($this->once())->method('send')->with($this->callback($callback));
+ $address = new Address('me@example.com');
+ $mailer = new Mailer($transport);
+ $mailer->setFromAddressOverride('no-reply@example.com');
+ $mailer->send('to@example.com', $address, 'subject', 'body', null, 'reply-to@example.com');
+ }
+
/**
* Test bad to address.
*
--
GitLab