From fa28a69d855a077087eb406c0db1a2fa40131c45 Mon Sep 17 00:00:00 2001 From: Clement Escoffier Date: Wed, 16 Oct 2024 10:49:20 +0200 Subject: [PATCH] Add option to skip logging invalid recipients when sending emails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- extensions/mailer/deployment/pom.xml | 5 ++ .../io/quarkus/mailer/InvalidEmailTest.java | 82 ++++++++++++++++++ .../mailer/LoggedInvalidEmailTest.java | 86 +++++++++++++++++++ .../mailer/runtime/MailerRuntimeConfig.java | 9 ++ .../io/quarkus/mailer/runtime/Mailers.java | 6 +- .../mailer/runtime/MockMailboxImpl.java | 9 ++ .../mailer/runtime/MutinyMailerImpl.java | 41 ++++++++- .../mailer/runtime/MailerImplTest.java | 2 +- .../runtime/MailerWithMultipartImplTest.java | 2 +- .../mailer/runtime/MockMailerImplTest.java | 2 +- 10 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 extensions/mailer/deployment/src/test/java/io/quarkus/mailer/InvalidEmailTest.java create mode 100644 extensions/mailer/deployment/src/test/java/io/quarkus/mailer/LoggedInvalidEmailTest.java diff --git a/extensions/mailer/deployment/pom.xml b/extensions/mailer/deployment/pom.xml index 40145227a5c34..5a1a7d602f460 100644 --- a/extensions/mailer/deployment/pom.xml +++ b/extensions/mailer/deployment/pom.xml @@ -41,6 +41,11 @@ quarkus-junit5-internal test + + org.assertj + assertj-core + test + diff --git a/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/InvalidEmailTest.java b/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/InvalidEmailTest.java new file mode 100644 index 0000000000000..def8ee6565ae0 --- /dev/null +++ b/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/InvalidEmailTest.java @@ -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 to = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List cc = List.of(); + List 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 cc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List to = List.of(); + List 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 bcc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List to = List.of(); + List 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 send(List to, List cc, List bcc) { + Mail mail = new Mail() + .setTo(to) + .setCc(cc) + .setBcc(bcc) + .setSubject("Test") + .setText("Hello!"); + return mailer.send(mail); + } + } +} diff --git a/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/LoggedInvalidEmailTest.java b/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/LoggedInvalidEmailTest.java new file mode 100644 index 0000000000000..3f374361ea027 --- /dev/null +++ b/extensions/mailer/deployment/src/test/java/io/quarkus/mailer/LoggedInvalidEmailTest.java @@ -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 to = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List cc = List.of(); + List 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 alid@quarkus.io") + .hasMessageNotContaining("@text.io"); + Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0); + } + + @Test + public void testInvalidCC() { + List cc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List to = List.of(); + List 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 alid@quarkus.io") + .hasMessageNotContaining("@text.io"); + Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0); + } + + @Test + public void testInvalidBCC() { + List bcc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io"); + List to = List.of(); + List 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 alid@quarkus.io") + .hasMessageNotContaining("@text.io"); + Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0); + } + + @Singleton + static class Sender { + + @Inject + ReactiveMailer mailer; + + Uni send(List to, List cc, List bcc) { + Mail mail = new Mail() + .setTo(to) + .setCc(cc) + .setBcc(bcc) + .setSubject("Test") + .setText("Hello!"); + return mailer.send(mail); + } + } +} diff --git a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MailerRuntimeConfig.java b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MailerRuntimeConfig.java index 84cce9d833aae..c38c447db3773 100644 --- a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MailerRuntimeConfig.java +++ b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MailerRuntimeConfig.java @@ -272,4 +272,13 @@ public class MailerRuntimeConfig { */ @ConfigItem(defaultValue = "false") public boolean logRejectedRecipients = false; + + /** + * Log invalid recipients as warnings. + *

+ * 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; } diff --git a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/Mailers.java b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/Mailers.java index 02d60a4504b56..c9a6628b0083d 100644 --- a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/Mailers.java +++ b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/Mailers.java @@ -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) { @@ -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); diff --git a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MockMailboxImpl.java b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MockMailboxImpl.java index e20e8bb09debd..d36044e702474 100644 --- a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MockMailboxImpl.java +++ b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MockMailboxImpl.java @@ -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. @@ -22,22 +23,30 @@ public class MockMailboxImpl implements MockMailbox { Uni 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); diff --git a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MutinyMailerImpl.java b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MutinyMailerImpl.java index b6baa1a56b75e..d09082cec1358 100644 --- a/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MutinyMailerImpl.java +++ b/extensions/mailer/runtime/src/main/java/io/quarkus/mailer/runtime/MutinyMailerImpl.java @@ -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; @@ -47,11 +48,13 @@ public class MutinyMailerImpl implements ReactiveMailer { private final List 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 approvedRecipients, - boolean logRejectedRecipients) { + boolean logRejectedRecipients, boolean logInvalidRecipients) { this.vertx = vertx; this.client = client; this.mockMailbox = mockMailbox; @@ -60,6 +63,7 @@ public class MutinyMailerImpl implements ReactiveMailer { this.mock = mock; this.approvedRecipients = approvedRecipients; this.logRejectedRecipients = logRejectedRecipients; + this.logInvalidRecipients = logInvalidRecipients; } @Override @@ -149,6 +153,11 @@ private Uni 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()); @@ -177,13 +186,39 @@ private Uni 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 to, List cc, List 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> headers) { MultiMap mm = MultiMap.caseInsensitiveMultiMap(); headers.forEach(mm::add); diff --git a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerImplTest.java b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerImplTest.java index c36249fb5028e..a312f705a026e 100644 --- a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerImplTest.java +++ b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerImplTest.java @@ -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(); } diff --git a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerWithMultipartImplTest.java b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerWithMultipartImplTest.java index 9800244d53688..655a34c20cc24 100644 --- a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerWithMultipartImplTest.java +++ b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerWithMultipartImplTest.java @@ -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(); } diff --git a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MockMailerImplTest.java b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MockMailerImplTest.java index 453460c540720..ac0ae1a3c714c 100644 --- a/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MockMailerImplTest.java +++ b/extensions/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MockMailerImplTest.java @@ -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