Skip to content

refactor: use IEmailValidator.isValid instead of IMailer.validateEmailAddress #55557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kesselb merged 1 commit into from
Oct 7, 2025
Merged

refactor: use IEmailValidator.isValid instead of IMailer.validateEmailAddress #55557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\L10N\IFactory as L10NFactory;
use OCP\Mail\Headers\AutoSubmitted;
use OCP\Mail\IEMailTemplate;
use OCP\Mail\IEmailValidator;
use OCP\Mail\IMailer;
use OCP\Util;
use Psr\Log\LoggerInterface;
Expand All @@ -39,6 +40,7 @@ public function __construct(
LoggerInterface $logger,
L10NFactory $l10nFactory,
IURLGenerator $urlGenerator,
private IEmailValidator $emailValidator,
) {
parent::__construct($logger, $l10nFactory, $urlGenerator, $config);
}
Expand Down Expand Up @@ -96,7 +98,7 @@ public function send(VEvent $vevent,
$template->addFooter();

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

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

if (!$this->mailer->validateMailAddress($organizerEMail)) {
if (!$this->emailValidator->isValid($organizerEMail)) {
return null;
}

Expand Down Expand Up @@ -251,7 +253,7 @@ private function getAllEMailAddressesFromEvent(VEvent $vevent):array {
foreach ($emailAddressesOfDelegates as $addressesOfDelegate) {
if (strcasecmp($addressesOfDelegate, 'mailto:') === 0) {
$delegateEmail = substr($addressesOfDelegate, 7);
if ($this->mailer->validateMailAddress($delegateEmail)) {
if ($this->emailValidator->isValid($delegateEmail)) {
$emailAddresses[$delegateEmail] = [];
}
}
Expand Down Expand Up @@ -311,7 +313,7 @@ private function getEMailAddressOfAttendee(VObject\Property $attendee): ?string
return null;
}
$attendeeEMail = substr($attendee->getValue(), 7);
if (!$this->mailer->validateMailAddress($attendeeEMail)) {
if (!$this->emailValidator->isValid($attendeeEMail)) {
return null;
}

Expand Down
4 changes: 3 additions & 1 deletion apps/dav/lib/CalDAV/Schedule/IMipPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCP\Defaults;
use OCP\IAppConfig;
use OCP\IUserSession;
use OCP\Mail\IEmailValidator;
use OCP\Mail\IMailer;
use OCP\Mail\Provider\Address;
use OCP\Mail\Provider\Attachment;
Expand Down Expand Up @@ -63,6 +64,7 @@ public function __construct(
private IMipService $imipService,
private EventComparisonService $eventComparisonService,
private IMailManager $mailManager,
private IEmailValidator $emailValidator,
) {
parent::__construct('');
}
Expand Down Expand Up @@ -119,7 +121,7 @@ public function schedule(Message $iTipMessage) {

// Strip off mailto:
$recipient = substr($iTipMessage->recipient, 7);
if (!$this->mailer->validateMailAddress($recipient)) {
if (!$this->emailValidator->isValid($recipient)) {
// Nothing to send if the recipient doesn't have a valid email address
$iTipMessage->scheduleStatus = '5.0; EMail delivery failed';
return;
Expand Down
6 changes: 3 additions & 3 deletions apps/dav/lib/Listener/CalendarContactInteractionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use OCP\EventDispatcher\IEventListener;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Mail\IMailer;
use OCP\Mail\IEmailValidator;
use Psr\Log\LoggerInterface;
use Sabre\VObject\Component\VEvent;
use Sabre\VObject\Parameter;
Expand All @@ -36,7 +36,7 @@ public function __construct(
private IEventDispatcher $dispatcher,
private IUserSession $userSession,
private Principal $principalConnector,
private IMailer $mailer,
private IEmailValidator $emailValidator,
private LoggerInterface $logger,
) {
}
Expand Down Expand Up @@ -129,7 +129,7 @@ private function emitFromObject(VEvent $vevent, IUser $user): void {
continue;
}
$email = substr($mailTo, strlen('mailto:'));
if (!$this->mailer->validateMailAddress($email)) {
if (!$this->emailValidator->isValid($email)) {
// This really isn't a valid email
continue;
}
Expand Down
3 changes: 2 additions & 1 deletion apps/dav/lib/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ public function __construct(
$userSession,
\OCP\Server::get(IMipService::class),
\OCP\Server::get(EventComparisonService::class),
\OCP\Server::get(\OCP\Mail\Provider\IManager::class)
\OCP\Server::get(\OCP\Mail\Provider\IManager::class),
\OCP\Server::get(\OCP\Mail\IEmailValidator::class),
));
}
$this->server->addPlugin(new \OCA\DAV\CalDAV\Search\SearchPlugin());
Expand Down
36 changes: 4 additions & 32 deletions apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
use OCP\Util;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\VObject\Component\VCalendar;
use Test\Traits\EmailValidatorTrait;

class EmailProviderTest extends AbstractNotificationProviderTestCase {
use EmailValidatorTrait;
public const USER_EMAIL = 'frodo@hobb.it';
private IMailer&MockObject $mailer;

Expand All @@ -32,7 +34,8 @@ protected function setUp(): void {
$this->mailer,
$this->logger,
$this->l10nFactory,
$this->urlGenerator
$this->urlGenerator,
$this->getEmailValidatorWithStrictEmailCheck(),
);
}

Expand Down Expand Up @@ -93,15 +96,6 @@ public function testSendWithoutAttendees():void {
$template2
);

$this->mailer->expects($this->exactly(4))
->method('validateMailAddress')
->willReturnMap([
['uid1@example.com', true],
['uid2@example.com', true],
['uid3@example.com', true],
['invalid', false],
]);

$this->mailer->expects($this->exactly(3))
->method('createMessage')
->with()
Expand Down Expand Up @@ -189,17 +183,6 @@ public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
$template1,
$template2,
);
$this->mailer->expects($this->atLeastOnce())
->method('validateMailAddress')
->willReturnMap([
['foo1@example.org', true],
['foo3@example.org', true],
['foo4@example.org', true],
['uid1@example.com', true],
['uid2@example.com', true],
['uid3@example.com', true],
['invalid', false],
]);
$this->mailer->expects($this->exactly(6))
->method('createMessage')
->with()
Expand Down Expand Up @@ -277,17 +260,6 @@ public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
->willReturnOnConsecutiveCalls(
$template1,
);
$this->mailer->expects($this->atLeastOnce())
->method('validateMailAddress')
->willReturnMap([
['foo1@example.org', true],
['foo3@example.org', true],
['foo4@example.org', true],
['uid1@example.com', true],
['uid2@example.com', true],
['uid3@example.com', true],
['invalid', false],
]);
$this->mailer->expects($this->exactly(2))
->method('createMessage')
->with()
Expand Down
5 changes: 3 additions & 2 deletions apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@
use Sabre\VObject\Property\ICalendar\CalAddress;
use Symfony\Component\Mime\Email;
use Test\TestCase;
use Test\Traits\EmailValidatorTrait;

class IMipPluginCharsetTest extends TestCase {
use EmailValidatorTrait;
// Dependencies
private Defaults&MockObject $defaults;
private IAppConfig&MockObject $appConfig;
Expand Down Expand Up @@ -102,8 +104,6 @@ protected function setUp(): void {
$this->mailer = $this->createMock(IMailer::class);
$this->mailer->method('createMessage')
->willReturn($message);
$this->mailer->method('validateMailAddress')
->willReturn(true);
$this->logger = new NullLogger();
$this->defaults = $this->createMock(Defaults::class);
$this->defaults->method('getName')
Expand All @@ -125,6 +125,7 @@ protected function setUp(): void {
$this->imipService,
$this->eventComparisonService,
$this->mailManager,
$this->getEmailValidatorWithStrictEmailCheck(),
);

// ITipMessage
Expand Down
47 changes: 5 additions & 42 deletions apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use Sabre\VObject\Component\VEvent;
use Sabre\VObject\ITip\Message;
use Test\TestCase;
use Test\Traits\EmailValidatorTrait;
use function array_merge;

interface IMailServiceMock extends IMailService, IMailMessageSend {
Expand All @@ -38,6 +39,8 @@ interface IMailServiceMock extends IMailService, IMailMessageSend {
}

class IMipPluginTest extends TestCase {
use EmailValidatorTrait;

private IMessage&MockObject $mailMessage;
private IMailer&MockObject $mailer;
private IEMailTemplate&MockObject $emailTemplate;
Expand Down Expand Up @@ -107,6 +110,7 @@ protected function setUp(): void {
$this->service,
$this->eventComparisonService,
$this->mailManager,
$this->getEmailValidatorWithStrictEmailCheck(),
);
}

Expand Down Expand Up @@ -173,10 +177,6 @@ public function testParsingSingle(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
Expand Down Expand Up @@ -280,10 +280,6 @@ public function testAttendeeIsResource(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('the-shire@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
Expand Down Expand Up @@ -358,10 +354,6 @@ public function testAttendeeIsCircle(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('circle+82utEV1Fle8wvxndZLK5TVAPtxj8IIe@middle.earth')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['new' => [$newVevent], 'old' => null]);
Expand Down Expand Up @@ -463,10 +455,6 @@ public function testParsingRecurrence(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['old' => [] ,'new' => [$newVevent]]);
Expand Down Expand Up @@ -541,15 +529,11 @@ public function testEmailValidationFailed(): void {
$message->message->VEVENT->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE']);
$message->sender = 'mailto:gandalf@wiz.ard';
$message->senderName = 'Mr. Wizard';
$message->recipient = 'mailto:' . 'frodo@hobb.it';
$message->recipient = 'mailto:' . 'frodo@@hobb.it';

$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(false);

$this->plugin->schedule($message);
$this->assertEquals('5.0', $message->getScheduleStatus());
Expand Down Expand Up @@ -598,10 +582,6 @@ public function testFailedDelivery(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['old' => [] ,'new' => [$newVevent]]);
Expand Down Expand Up @@ -755,11 +735,6 @@ public function testMailProviderSend(): void {
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['old' => [] ,'new' => [$event]]);
// construct mail mock returns
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
// construct mail provider mock returns
$this->mailService
->method('initiateMessage')
Expand Down Expand Up @@ -819,10 +794,6 @@ public function testMailProviderDisabled(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]);
Expand Down Expand Up @@ -917,10 +888,6 @@ public function testNoOldEvent(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->with($newVCalendar, null)
Expand Down Expand Up @@ -1014,10 +981,6 @@ public function testNoButtons(): void {
$this->service->expects(self::once())
->method('getLastOccurrence')
->willReturn(1496912700);
$this->mailer->expects(self::once())
->method('validateMailAddress')
->with('frodo@hobb.it')
->willReturn(true);
$this->eventComparisonService->expects(self::once())
->method('findModified')
->with($newVCalendar, null)
Expand Down
Loading
Loading