Skip to content

Commit db3e845

Browse files
kesselbkesselb
authored
Merge pull request #55557 from nextcloud/refactor/use-iemailaddressvalidator
refactor: use IEmailValidator.isValid instead of IMailer.validateEmailAddress
2 parents fb0b56d + 83a6917 commit db3e845

20 files changed

Lines changed: 78 additions & 185 deletions

File tree

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCP\L10N\IFactory as L10NFactory;
1717
use OCP\Mail\Headers\AutoSubmitted;
1818
use OCP\Mail\IEMailTemplate;
19+
use OCP\Mail\IEmailValidator;
1920
use OCP\Mail\IMailer;
2021
use OCP\Util;
2122
use Psr\Log\LoggerInterface;
@@ -39,6 +40,7 @@ public function __construct(
3940
LoggerInterface $logger,
4041
L10NFactory $l10nFactory,
4142
IURLGenerator $urlGenerator,
43+
private IEmailValidator $emailValidator,
4244
) {
4345
parent::__construct($logger, $l10nFactory, $urlGenerator, $config);
4446
}
@@ -96,7 +98,7 @@ public function send(VEvent $vevent,
9698
$template->addFooter();
9799

98100
foreach ($emailAddresses as $emailAddress) {
99-
if (!$this->mailer->validateMailAddress($emailAddress)) {
101+
if (!$this->emailValidator->isValid($emailAddress)) {
100102
$this->logger->error('Email address {address} for reminder notification is incorrect', ['app' => 'dav', 'address' => $emailAddress]);
101103
continue;
102104
}
@@ -180,7 +182,7 @@ private function getOrganizerEMailAndNameFromEvent(VEvent $vevent):?array {
180182

181183
$organizerEMail = substr($organizer->getValue(), 7);
182184

183-
if (!$this->mailer->validateMailAddress($organizerEMail)) {
185+
if (!$this->emailValidator->isValid($organizerEMail)) {
184186
return null;
185187
}
186188

@@ -251,7 +253,7 @@ private function getAllEMailAddressesFromEvent(VEvent $vevent):array {
251253
foreach ($emailAddressesOfDelegates as $addressesOfDelegate) {
252254
if (strcasecmp($addressesOfDelegate, 'mailto:') === 0) {
253255
$delegateEmail = substr($addressesOfDelegate, 7);
254-
if ($this->mailer->validateMailAddress($delegateEmail)) {
256+
if ($this->emailValidator->isValid($delegateEmail)) {
255257
$emailAddresses[$delegateEmail] = [];
256258
}
257259
}
@@ -311,7 +313,7 @@ private function getEMailAddressOfAttendee(VObject\Property $attendee): ?string
311313
return null;
312314
}
313315
$attendeeEMail = substr($attendee->getValue(), 7);
314-
if (!$this->mailer->validateMailAddress($attendeeEMail)) {
316+
if (!$this->emailValidator->isValid($attendeeEMail)) {
315317
return null;
316318
}
317319

‎apps/dav/lib/CalDAV/Schedule/IMipPlugin.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCP\Defaults;
1515
use OCP\IAppConfig;
1616
use OCP\IUserSession;
17+
use OCP\Mail\IEmailValidator;
1718
use OCP\Mail\IMailer;
1819
use OCP\Mail\Provider\Address;
1920
use OCP\Mail\Provider\Attachment;
@@ -63,6 +64,7 @@ public function __construct(
6364
private IMipService $imipService,
6465
private EventComparisonService $eventComparisonService,
6566
private IMailManager $mailManager,
67+
private IEmailValidator $emailValidator,
6668
) {
6769
parent::__construct('');
6870
}
@@ -119,7 +121,7 @@ public function schedule(Message $iTipMessage) {
119121

120122
// Strip off mailto:
121123
$recipient = substr($iTipMessage->recipient, 7);
122-
if (!$this->mailer->validateMailAddress($recipient)) {
124+
if (!$this->emailValidator->isValid($recipient)) {
123125
// Nothing to send if the recipient doesn't have a valid email address
124126
$iTipMessage->scheduleStatus = '5.0; EMail delivery failed';
125127
return;

‎apps/dav/lib/Listener/CalendarContactInteractionListener.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use OCP\EventDispatcher\IEventListener;
1919
use OCP\IUser;
2020
use OCP\IUserSession;
21-
use OCP\Mail\IMailer;
21+
use OCP\Mail\IEmailValidator;
2222
use Psr\Log\LoggerInterface;
2323
use Sabre\VObject\Component\VEvent;
2424
use Sabre\VObject\Parameter;
@@ -36,7 +36,7 @@ public function __construct(
3636
private IEventDispatcher $dispatcher,
3737
private IUserSession $userSession,
3838
private Principal $principalConnector,
39-
private IMailer $mailer,
39+
private IEmailValidator $emailValidator,
4040
private LoggerInterface $logger,
4141
) {
4242
}
@@ -129,7 +129,7 @@ private function emitFromObject(VEvent $vevent, IUser $user): void {
129129
continue;
130130
}
131131
$email = substr($mailTo, strlen('mailto:'));
132-
if (!$this->mailer->validateMailAddress($email)) {
132+
if (!$this->emailValidator->isValid($email)) {
133133
// This really isn't a valid email
134134
continue;
135135
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ public function __construct(
350350
$userSession,
351351
\OCP\Server::get(IMipService::class),
352352
\OCP\Server::get(EventComparisonService::class),
353-
\OCP\Server::get(\OCP\Mail\Provider\IManager::class)
353+
\OCP\Server::get(\OCP\Mail\Provider\IManager::class),
354+
\OCP\Server::get(\OCP\Mail\IEmailValidator::class),
354355
));
355356
}
356357
$this->server->addPlugin(new \OCA\DAV\CalDAV\Search\SearchPlugin());

‎apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
use OCP\Util;
1818
use PHPUnit\Framework\MockObject\MockObject;
1919
use Sabre\VObject\Component\VCalendar;
20+
use Test\Traits\EmailValidatorTrait;
2021

2122
class EmailProviderTest extends AbstractNotificationProviderTestCase {
23+
use EmailValidatorTrait;
2224
public const USER_EMAIL = '[email protected]';
2325
private IMailer&MockObject $mailer;
2426

@@ -32,7 +34,8 @@ protected function setUp(): void {
3234
$this->mailer,
3335
$this->logger,
3436
$this->l10nFactory,
35-
$this->urlGenerator
37+
$this->urlGenerator,
38+
$this->getEmailValidatorWithStrictEmailCheck(),
3639
);
3740
}
3841

@@ -93,15 +96,6 @@ public function testSendWithoutAttendees():void {
9396
$template2
9497
);
9598

96-
$this->mailer->expects($this->exactly(4))
97-
->method('validateMailAddress')
98-
->willReturnMap([
99-
['[email protected]', true],
100-
['[email protected]', true],
101-
['[email protected]', true],
102-
['invalid', false],
103-
]);
104-
10599
$this->mailer->expects($this->exactly(3))
106100
->method('createMessage')
107101
->with()
@@ -189,17 +183,6 @@ public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
189183
$template1,
190184
$template2,
191185
);
192-
$this->mailer->expects($this->atLeastOnce())
193-
->method('validateMailAddress')
194-
->willReturnMap([
195-
['[email protected]', true],
196-
['[email protected]', true],
197-
['[email protected]', true],
198-
['[email protected]', true],
199-
['[email protected]', true],
200-
['[email protected]', true],
201-
['invalid', false],
202-
]);
203186
$this->mailer->expects($this->exactly(6))
204187
->method('createMessage')
205188
->with()
@@ -277,17 +260,6 @@ public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
277260
->willReturnOnConsecutiveCalls(
278261
$template1,
279262
);
280-
$this->mailer->expects($this->atLeastOnce())
281-
->method('validateMailAddress')
282-
->willReturnMap([
283-
['[email protected]', true],
284-
['[email protected]', true],
285-
['[email protected]', true],
286-
['[email protected]', true],
287-
['[email protected]', true],
288-
['[email protected]', true],
289-
['invalid', false],
290-
]);
291263
$this->mailer->expects($this->exactly(2))
292264
->method('createMessage')
293265
->with()

‎apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@
3838
use Sabre\VObject\Property\ICalendar\CalAddress;
3939
use Symfony\Component\Mime\Email;
4040
use Test\TestCase;
41+
use Test\Traits\EmailValidatorTrait;
4142

4243
class IMipPluginCharsetTest extends TestCase {
44+
use EmailValidatorTrait;
4345
// Dependencies
4446
private Defaults&MockObject $defaults;
4547
private IAppConfig&MockObject $appConfig;
@@ -102,8 +104,6 @@ protected function setUp(): void {
102104
$this->mailer = $this->createMock(IMailer::class);
103105
$this->mailer->method('createMessage')
104106
->willReturn($message);
105-
$this->mailer->method('validateMailAddress')
106-
->willReturn(true);
107107
$this->logger = new NullLogger();
108108
$this->defaults = $this->createMock(Defaults::class);
109109
$this->defaults->method('getName')
@@ -125,6 +125,7 @@ protected function setUp(): void {
125125
$this->imipService,
126126
$this->eventComparisonService,
127127
$this->mailManager,
128+
$this->getEmailValidatorWithStrictEmailCheck(),
128129
);
129130

130131
// ITipMessage

‎apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php‎

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use Sabre\VObject\Component\VEvent;
3131
use Sabre\VObject\ITip\Message;
3232
use Test\TestCase;
33+
use Test\Traits\EmailValidatorTrait;
3334
use function array_merge;
3435

3536
interface IMailServiceMock extends IMailService, IMailMessageSend {
@@ -38,6 +39,8 @@ interface IMailServiceMock extends IMailService, IMailMessageSend {
3839
}
3940

4041
class IMipPluginTest extends TestCase {
42+
use EmailValidatorTrait;
43+
4144
private IMessage&MockObject $mailMessage;
4245
private IMailer&MockObject $mailer;
4346
private IEMailTemplate&MockObject $emailTemplate;
@@ -107,6 +110,7 @@ protected function setUp(): void {
107110
$this->service,
108111
$this->eventComparisonService,
109112
$this->mailManager,
113+
$this->getEmailValidatorWithStrictEmailCheck(),
110114
);
111115
}
112116

@@ -173,10 +177,6 @@ public function testParsingSingle(): void {
173177
$this->service->expects(self::once())
174178
->method('getLastOccurrence')
175179
->willReturn(1496912700);
176-
$this->mailer->expects(self::once())
177-
->method('validateMailAddress')
178-
->with('[email protected]')
179-
->willReturn(true);
180180
$this->eventComparisonService->expects(self::once())
181181
->method('findModified')
182182
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
@@ -280,10 +280,6 @@ public function testAttendeeIsResource(): void {
280280
$this->service->expects(self::once())
281281
->method('getLastOccurrence')
282282
->willReturn(1496912700);
283-
$this->mailer->expects(self::once())
284-
->method('validateMailAddress')
285-
->with('[email protected]')
286-
->willReturn(true);
287283
$this->eventComparisonService->expects(self::once())
288284
->method('findModified')
289285
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
@@ -358,10 +354,6 @@ public function testAttendeeIsCircle(): void {
358354
$this->service->expects(self::once())
359355
->method('getLastOccurrence')
360356
->willReturn(1496912700);
361-
$this->mailer->expects(self::once())
362-
->method('validateMailAddress')
363-
->with('[email protected]')
364-
->willReturn(true);
365357
$this->eventComparisonService->expects(self::once())
366358
->method('findModified')
367359
->willReturn(['new' => [$newVevent], 'old' => null]);
@@ -463,10 +455,6 @@ public function testParsingRecurrence(): void {
463455
$this->service->expects(self::once())
464456
->method('getLastOccurrence')
465457
->willReturn(1496912700);
466-
$this->mailer->expects(self::once())
467-
->method('validateMailAddress')
468-
->with('[email protected]')
469-
->willReturn(true);
470458
$this->eventComparisonService->expects(self::once())
471459
->method('findModified')
472460
->willReturn(['old' => [] ,'new' => [$newVevent]]);
@@ -541,15 +529,11 @@ public function testEmailValidationFailed(): void {
541529
$message->message->VEVENT->add('ATTENDEE', 'mailto:' . '[email protected]', ['RSVP' => 'TRUE']);
542530
$message->sender = 'mailto:[email protected]';
543531
$message->senderName = 'Mr. Wizard';
544-
$message->recipient = 'mailto:' . '[email protected]';
532+
$message->recipient = 'mailto:' . 'frodo@@hobb.it';
545533

546534
$this->service->expects(self::once())
547535
->method('getLastOccurrence')
548536
->willReturn(1496912700);
549-
$this->mailer->expects(self::once())
550-
->method('validateMailAddress')
551-
->with('[email protected]')
552-
->willReturn(false);
553537

554538
$this->plugin->schedule($message);
555539
$this->assertEquals('5.0', $message->getScheduleStatus());
@@ -598,10 +582,6 @@ public function testFailedDelivery(): void {
598582
$this->service->expects(self::once())
599583
->method('getLastOccurrence')
600584
->willReturn(1496912700);
601-
$this->mailer->expects(self::once())
602-
->method('validateMailAddress')
603-
->with('[email protected]')
604-
->willReturn(true);
605585
$this->eventComparisonService->expects(self::once())
606586
->method('findModified')
607587
->willReturn(['old' => [] ,'new' => [$newVevent]]);
@@ -755,11 +735,6 @@ public function testMailProviderSend(): void {
755735
$this->eventComparisonService->expects(self::once())
756736
->method('findModified')
757737
->willReturn(['old' => [] ,'new' => [$event]]);
758-
// construct mail mock returns
759-
$this->mailer->expects(self::once())
760-
->method('validateMailAddress')
761-
->with('[email protected]')
762-
->willReturn(true);
763738
// construct mail provider mock returns
764739
$this->mailService
765740
->method('initiateMessage')
@@ -819,10 +794,6 @@ public function testMailProviderDisabled(): void {
819794
$this->service->expects(self::once())
820795
->method('getLastOccurrence')
821796
->willReturn(1496912700);
822-
$this->mailer->expects(self::once())
823-
->method('validateMailAddress')
824-
->with('[email protected]')
825-
->willReturn(true);
826797
$this->eventComparisonService->expects(self::once())
827798
->method('findModified')
828799
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
@@ -917,10 +888,6 @@ public function testNoOldEvent(): void {
917888
$this->service->expects(self::once())
918889
->method('getLastOccurrence')
919890
->willReturn(1496912700);
920-
$this->mailer->expects(self::once())
921-
->method('validateMailAddress')
922-
->with('[email protected]')
923-
->willReturn(true);
924891
$this->eventComparisonService->expects(self::once())
925892
->method('findModified')
926893
->with($newVCalendar, null)
@@ -1014,10 +981,6 @@ public function testNoButtons(): void {
1014981
$this->service->expects(self::once())
1015982
->method('getLastOccurrence')
1016983
->willReturn(1496912700);
1017-
$this->mailer->expects(self::once())
1018-
->method('validateMailAddress')
1019-
->with('[email protected]')
1020-
->willReturn(true);
1021984
$this->eventComparisonService->expects(self::once())
1022985
->method('findModified')
1023986
->with($newVCalendar, null)

0 commit comments

Comments
 (0)