Skip to content

Commit

Permalink
Add option to skip logging invalid recipients when sending emails
Browse files Browse the repository at this point in the history
This commits detects and prevents logging invalid recipients before delegating to the underlying SMTP client, avoiding the overhead of processing attachments for emails that won’t be sent as well as acquiring an unnecessary connection.

When the option is enabled (default), the invalid recipients is not logged and not part of the thrown exception.
  • Loading branch information
cescoffier committed Oct 16, 2024
1 parent fe164e0 commit fa28a69
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 8 deletions.
5 changes: 5 additions & 0 deletions extensions/mailer/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@
<artifactId>quarkus-junit5-internal</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

</dependencies>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package io.quarkus.mailer;

import java.time.Duration;
import java.util.List;

import jakarta.inject.Inject;
import jakarta.inject.Singleton;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.mailer.reactive.ReactiveMailer;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.mutiny.Uni;

public class InvalidEmailTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot(root -> root
.addClasses(Sender.class));

@Inject
MockMailbox mockMailbox;

@Inject
Sender sender;

@Test
public void testInvalidTo() {
List<String> to = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> cc = List.of();
List<String> bcc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email, an email address is invalid")
.hasMessageNotContaining("@");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Test
public void testInvalidCC() {
List<String> cc = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> to = List.of();
List<String> bcc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email, an email address is invalid")
.hasMessageNotContaining("@");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Test
public void testInvalidBCC() {
List<String> bcc = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> to = List.of();
List<String> cc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email, an email address is invalid")
.hasMessageNotContaining("@");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Singleton
static class Sender {

@Inject
ReactiveMailer mailer;

Uni<Void> send(List<String> to, List<String> cc, List<String> bcc) {
Mail mail = new Mail()
.setTo(to)
.setCc(cc)
.setBcc(bcc)
.setSubject("Test")
.setText("Hello!");
return mailer.send(mail);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package io.quarkus.mailer;

import java.time.Duration;
import java.util.List;

import jakarta.inject.Inject;
import jakarta.inject.Singleton;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.mailer.reactive.ReactiveMailer;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.mutiny.Uni;

public class LoggedInvalidEmailTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot(root -> root
.addClasses(Sender.class))
.overrideRuntimeConfigKey("quarkus.mailer.log-invalid-recipients", "true");

@Inject
MockMailbox mockMailbox;

@Inject
Sender sender;

@Test
public void testInvalidTo() {
List<String> to = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> cc = List.of();
List<String> bcc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email")
.hasStackTraceContaining("inv [email protected]")
.hasMessageNotContaining("@text.io");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Test
public void testInvalidCC() {
List<String> cc = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> to = List.of();
List<String> bcc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email")
.hasStackTraceContaining("inv [email protected]")
.hasMessageNotContaining("@text.io");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Test
public void testInvalidBCC() {
List<String> bcc = List.of("[email protected]", "inv [email protected]", "[email protected]");
List<String> to = List.of();
List<String> cc = List.of();
Assertions.assertThatThrownBy(() -> sender.send(to, cc, bcc).await().atMost(Duration.ofSeconds(5)))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unable to send an email")
.hasStackTraceContaining("inv [email protected]")
.hasMessageNotContaining("@text.io");
Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
}

@Singleton
static class Sender {

@Inject
ReactiveMailer mailer;

Uni<Void> send(List<String> to, List<String> cc, List<String> bcc) {
Mail mail = new Mail()
.setTo(to)
.setCc(cc)
.setBcc(bcc)
.setSubject("Test")
.setText("Hello!");
return mailer.send(mail);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,13 @@ public class MailerRuntimeConfig {
*/
@ConfigItem(defaultValue = "false")
public boolean logRejectedRecipients = false;

/**
* Log invalid recipients as warnings.
* <p>
* If false, the invalid recipients will not be logged and the thrown exception will not contain the invalid email address.
*
*/
@ConfigItem(defaultValue = "false")
public boolean logInvalidRecipients = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ public Mailers(Vertx vertx, io.vertx.mutiny.core.Vertx mutinyVertx, MailersRunti
mailersRuntimeConfig.defaultMailer.mock.orElse(launchMode.isDevOrTest()),
mailersRuntimeConfig.defaultMailer.approvedRecipients.orElse(List.of()).stream()
.filter(Objects::nonNull).collect(Collectors.toList()),
mailersRuntimeConfig.defaultMailer.logRejectedRecipients));
mailersRuntimeConfig.defaultMailer.logRejectedRecipients,
mailersRuntimeConfig.defaultMailer.logInvalidRecipients));
}

for (String name : mailerSupport.namedMailers) {
Expand All @@ -97,7 +98,8 @@ public Mailers(Vertx vertx, io.vertx.mutiny.core.Vertx mutinyVertx, MailersRunti
namedMailerRuntimeConfig.mock.orElse(false),
namedMailerRuntimeConfig.approvedRecipients.orElse(List.of()).stream()
.filter(p -> p != null).collect(Collectors.toList()),
namedMailerRuntimeConfig.logRejectedRecipients));
namedMailerRuntimeConfig.logRejectedRecipients,
namedMailerRuntimeConfig.logInvalidRecipients));
}

this.clients = Collections.unmodifiableMap(localClients);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.quarkus.mailer.MockMailbox;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.mail.MailMessage;
import io.vertx.ext.mail.mailencoder.EmailAddress;

/**
* Mock mailbox bean, will be populated if mocking emails.
Expand All @@ -22,22 +23,30 @@ public class MockMailboxImpl implements MockMailbox {
Uni<Void> send(Mail email, MailMessage mailMessage) {
if (email.getTo() != null) {
for (String to : email.getTo()) {
validateEmailAddress(to);
send(email, mailMessage, to);
}
}
if (email.getCc() != null) {
for (String to : email.getCc()) {
validateEmailAddress(to);
send(email, mailMessage, to);
}
}
if (email.getBcc() != null) {
for (String to : email.getBcc()) {
validateEmailAddress(to);
send(email, mailMessage, to);
}
}
return Uni.createFrom().item(() -> null);
}

private void validateEmailAddress(String to) {
// Just here to validate the email address.
new EmailAddress(to);
}

private void send(Mail sentMail, MailMessage sentMailMessage, String to) {
sentMails.computeIfAbsent(to, k -> new ArrayList<>()).add(sentMail);
sentMailMessages.computeIfAbsent(to, k -> new ArrayList<>()).add(sentMailMessage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.vertx.core.file.OpenOptions;
import io.vertx.ext.mail.MailAttachment;
import io.vertx.ext.mail.MailMessage;
import io.vertx.ext.mail.mailencoder.EmailAddress;
import io.vertx.mutiny.core.Vertx;
import io.vertx.mutiny.core.file.AsyncFile;
import io.vertx.mutiny.ext.mail.MailClient;
Expand All @@ -47,11 +48,13 @@ public class MutinyMailerImpl implements ReactiveMailer {

private final List<Pattern> approvedRecipients;

private boolean logRejectedRecipients;
private final boolean logRejectedRecipients;

private final boolean logInvalidRecipients;

MutinyMailerImpl(Vertx vertx, MailClient client, MockMailboxImpl mockMailbox,
String from, String bounceAddress, boolean mock, List<Pattern> approvedRecipients,
boolean logRejectedRecipients) {
boolean logRejectedRecipients, boolean logInvalidRecipients) {
this.vertx = vertx;
this.client = client;
this.mockMailbox = mockMailbox;
Expand All @@ -60,6 +63,7 @@ public class MutinyMailerImpl implements ReactiveMailer {
this.mock = mock;
this.approvedRecipients = approvedRecipients;
this.logRejectedRecipients = logRejectedRecipients;
this.logInvalidRecipients = logInvalidRecipients;
}

@Override
Expand Down Expand Up @@ -149,6 +153,11 @@ private Uni<MailMessage> toMailMessage(Mail mail) {
message.setTo(mail.getTo());
message.setCc(mail.getCc());
message.setBcc(mail.getBcc());

// Validate that the email addresses are valid
// We do that early to avoid having to read attachments if an email is invalid
validate(mail.getTo(), mail.getCc(), mail.getBcc());

message.setSubject(mail.getSubject());
message.setText(mail.getText());
message.setHtml(mail.getHtml());
Expand Down Expand Up @@ -177,13 +186,39 @@ private Uni<MailMessage> toMailMessage(Mail mail) {
return Uni.createFrom().item(message);
}

return Uni.combine().all().unis(stages).combinedWith(res -> {
return Uni.combine().all().unis(stages).with(res -> {
message.setAttachment(attachments);
message.setInlineAttachment(inline);
return message;
});
}

private void validate(List<String> to, List<String> cc, List<String> bcc) {
try {
for (String email : to) {
new EmailAddress(email);
}
for (String email : cc) {
new EmailAddress(email);
}
for (String email : bcc) {
new EmailAddress(email);
}
} catch (IllegalArgumentException e) {
// One of the email addresses is invalid
if (logInvalidRecipients) {
// We are allowed to log the invalid email address
// The exception message contains the invalid email address.
LOGGER.warn("Unable to send an email", e);
throw new IllegalArgumentException("Unable to send an email", e);
} else {
// Do not print the invalid email address.
LOGGER.warn("Unable to send an email, an email address is invalid");
throw new IllegalArgumentException("Unable to send an email, an email address is invalid");
}
}
}

private MultiMap toMultimap(Map<String, List<String>> headers) {
MultiMap mm = MultiMap.caseInsensitiveMultiMap();
headers.forEach(mm::add);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void init() {
mailer = new MutinyMailerImpl(vertx,
MailClient.createShared(vertx,
new MailConfig().setPort(wiser.getServer().getPort())),
null, FROM, null, false, List.of(), false);
null, FROM, null, false, List.of(), false, false);

wiser.getMessages().clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static void stopWiser() {
void init() {
mailer = new MutinyMailerImpl(vertx, MailClient.createShared(vertx,
new MailConfig().setPort(wiser.getServer().getPort()).setMultiPartOnly(true)), null,
FROM, null, false, List.of(), false);
FROM, null, false, List.of(), false, false);

wiser.getMessages().clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static void stop() {
@BeforeEach
void init() {
mockMailbox = new MockMailboxImpl();
mailer = new MutinyMailerImpl(vertx, null, mockMailbox, FROM, null, true, List.of(), false);
mailer = new MutinyMailerImpl(vertx, null, mockMailbox, FROM, null, true, List.of(), false, false);
}

@Test
Expand Down

0 comments on commit fa28a69

Please sign in to comment.