Merge pull request quarkusio#43955 from cescoffier/mailer-no-log-inva…

…lid-recipient Add option to skip logging invalid recipients when sending emails
aloubyansky · Oct 19, 2024 · 744bc75 · 744bc75
2 parents 6e8ab9a + fa28a69
commit 744bc75
Show file tree

Hide file tree

Showing 10 changed files with 236 additions and 8 deletions.
diff --git a/extensions/mailer/deployment/pom.xml b/extensions/mailer/deployment/pom.xml
@@ -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>
 

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
@@ -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("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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);
+        }
+    }
+}
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
@@ -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("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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 alid@quarkus.io")
+                .hasMessageNotContaining("@text.io");
+        Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
+    }
+
+    @Test
+    public void testInvalidCC() {
+        List<String> cc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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 alid@quarkus.io")
+                .hasMessageNotContaining("@text.io");
+        Assertions.assertThat(mockMailbox.getTotalMessagesSent()).isEqualTo(0);
+    }
+
+    @Test
+    public void testInvalidBCC() {
+        List<String> bcc = List.of("clement@test.io", "inv alid@quarkus.io", "max@test.io");
+        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 alid@quarkus.io")
+                .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);
+        }
+    }
+}
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
@@ -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;
 }
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
@@ -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
@@ -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<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);

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
@@ -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<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;
@@ -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<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());
@@ -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);

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
@@ -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/...s/mailer/runtime/src/test/java/io/quarkus/mailer/runtime/MailerWithMultipartImplTest.java b/...s/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
@@ -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