(rocketmq-dashboard) branch rocketmq-studio updated: fix(alert): preserve comments in email recipient lists (#6922)

[lizhimin]

This is an automated email from the ASF dual-hosted git repository.

lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git


The following commit(s) were added to refs/heads/rocketmq-studio by this push:
     new 6ddbc6db5 fix(alert): preserve comments in email recipient lists 
(#6922)
6ddbc6db5 is described below

commit 6ddbc6db5d37d727758c4c286d818eb87b3011e8
Author: pilichoumao <99873428+pilichou...@users.noreply.github.com>
AuthorDate: Mon Oct 26 16:32:30 2026 +0800

    fix(alert): preserve comments in email recipient lists (#6922)
    
    * fix(alert): preserve comments in email recipient lists
---
 docs/studio-native-alerting-design.md              |   3 +-
 .../ops/alert/NotificationOutboxService.java       |  22 ++--
 .../ops/alert/NotificationEmailRecipientsTest.java | 122 +++++++++++++++++++++
 3 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/docs/studio-native-alerting-design.md 
b/docs/studio-native-alerting-design.md
index 542a7ad37..27648f72d 100644
--- a/docs/studio-native-alerting-design.md
+++ b/docs/studio-native-alerting-design.md
@@ -266,7 +266,8 @@ delivery is running. Deployments with slower receivers can 
tune `studio.alerting
 Email delivery uses Spring's standard SMTP configuration. Configure 
`STUDIO_ALERTING_SMTP_HOST`,
 `STUDIO_ALERTING_SMTP_PORT`, `STUDIO_ALERTING_SMTP_USERNAME`, 
`STUDIO_ALERTING_SMTP_PASSWORD`,
 `STUDIO_ALERTING_SMTP_AUTH`, and `STUDIO_ALERTING_SMTP_STARTTLS`; recipient 
addresses are configured in
-General Settings. Missing SMTP configuration leaves the outbox row retryable 
and records the delivery failure.
+General Settings. Commas and legacy semicolons separate addresses outside 
quoted display names and
+parenthesized comments; separators inside these parts belong to the address. 
Missing SMTP configuration leaves the outbox row retryable and records the 
delivery failure.
 
 Socket timeouts are bounded by default and tunable through 
`STUDIO_ALERTING_SMTP_CONNECTION_TIMEOUT` (5000 ms),
 `STUDIO_ALERTING_SMTP_READ_TIMEOUT` (10000 ms) and 
`STUDIO_ALERTING_SMTP_WRITE_TIMEOUT` (10000 ms). They matter
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/NotificationOutboxService.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/NotificationOutboxService.java
index 26e8b017f..1e754daaf 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/NotificationOutboxService.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/NotificationOutboxService.java
@@ -498,23 +498,31 @@ public class NotificationOutboxService {
 
     private static String[] parseRecipients(String raw) throws 
AddressException {
         List<String> recipients = new ArrayList<>();
-        // Split on the documented separators (comma, plus the legacy
-        // semicolon) only outside a quoted display name: a list like
-        // `"Doe, John" <j...@example.com>, o...@example.com` is valid RFC 822
-        // input, but a plain [,;] split broke it into unbalanced fragments
-        // that failed validation and drove the notification into retry
-        // instead of sending.
+        // Commas and legacy semicolons separate recipients only outside quoted
+        // strings and comments. Comments may nest and contain quoted pairs;
+        // keep each address intact for InternetAddress's strict validation.
         StringBuilder current = new StringBuilder();
         boolean quoted = false;
+        int commentDepth = 0;
         boolean escaped = false;
         for (int i = 0; i < raw.length(); i++) {
             char c = raw.charAt(i);
             if (escaped) {
                 current.append(c);
                 escaped = false;
-            } else if (quoted && c == '\\') {
+            } else if ((quoted || commentDepth > 0) && c == '\\') {
                 current.append(c);
                 escaped = true;
+            } else if (commentDepth > 0) {
+                current.append(c);
+                if (c == '(') {
+                    commentDepth++;
+                } else if (c == ')') {
+                    commentDepth--;
+                }
+            } else if (!quoted && c == '(') {
+                current.append(c);
+                commentDepth = 1;
             } else if (c == '"') {
                 current.append(c);
                 quoted = !quoted;
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/NotificationEmailRecipientsTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/NotificationEmailRecipientsTest.java
new file mode 100644
index 000000000..37dd620b0
--- /dev/null
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/NotificationEmailRecipientsTest.java
@@ -0,0 +1,122 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0.
+*/
+package org.apache.rocketmq.studio.ops.alert;
+
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
+import java.time.LocalDateTime;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.ScheduledExecutorService;
+import org.apache.rocketmq.studio.audit.OperationAuditService;
+import org.apache.rocketmq.studio.cluster.metrics.AlertingProperties;
+import org.apache.rocketmq.studio.common.domain.enums.AlertLevel;
+import 
org.apache.rocketmq.studio.persistence.entity.RmqAlertNotificationOutbox;
+import 
org.apache.rocketmq.studio.persistence.mapper.RmqAlertNotificationOutboxMapper;
+import org.apache.rocketmq.studio.settings.GeneralSettingsVO;
+import org.apache.rocketmq.studio.settings.SettingsRepository;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
+import org.springframework.mail.SimpleMailMessage;
+import org.springframework.mail.javamail.JavaMailSender;
+import org.springframework.web.client.RestTemplate;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+class NotificationEmailRecipientsTest {
+    private final RmqAlertNotificationOutboxMapper mapper = 
mock(RmqAlertNotificationOutboxMapper.class);
+    private final SettingsRepository settings = mock(SettingsRepository.class);
+    private final AlertRepository alerts = mock(AlertRepository.class);
+    private final JavaMailSender mailSender = mock(JavaMailSender.class);
+    private NotificationOutboxService service;
+
+    @BeforeEach
+    void setUpTest() {
+        RmqAlertNotificationOutbox row = new RmqAlertNotificationOutbox();
+        row.setId(8L);
+        row.setAlertId(9L);
+        row.setChannel("email");
+        row.setStatus("PENDING");
+        row.setAttemptCount(0);
+        when(mapper.findDispatchable(any(LocalDateTime.class), 
any(LocalDateTime.class), any(Integer.class)))
+                .thenReturn(List.of(row));
+        when(mapper.claimForDispatch(any(), any(LocalDateTime.class), 
any(LocalDateTime.class),
+                any(LocalDateTime.class), anyString(), 
anyInt())).thenReturn(1);
+        when(mapper.update(any(), any())).thenReturn(1);
+        
when(alerts.findAlertById(9L)).thenReturn(Optional.of(SystemAlertVO.builder().id(9L)
+                
.level(AlertLevel.warning).title("Lag").description("high").instanceId("local").build()));
+        service = new NotificationOutboxService(mapper, settings, 
mock(AlertSilenceService.class), alerts,
+                mock(OperationAuditService.class), new RestTemplate(), () -> 
mailSender,
+                new AlertingProperties(), 
mock(ScheduledExecutorService.class));
+    }
+
+    @AfterEach
+    void closeTest() {
+        service.closeHeartbeatExecutor();
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "o...@example.com, bac...@example.com",
+        "\"Ops, East\" <o...@example.com>; bac...@example.com",
+        "\"Ops; East\" <o...@example.com>, bac...@example.com",
+        "o...@example.com (Ops, on call), bac...@example.com",
+        "Ops (East; on call) <o...@example.com>; bac...@example.com",
+        "o...@example.com (Ops (East, primary)), bac...@example.com",
+        "o...@example.com (Ops \\(East, primary), bac...@example.com",
+        "o...@example.com (Ops \\), primary), bac...@example.com",
+        "o...@example.com (Ops \"East, primary), bac...@example.com",
+        "\"Ops (East), Team\" <o...@example.com>, bac...@example.com",
+        "\"Ops \\\"East, Team\" <o...@example.com>; bac...@example.com",
+        "\"Ops, Team\" (East; primary) <o...@example.com>; bac...@example.com"
+    })
+    void 
dispatchesCompleteAddressesWithoutSplittingCommentsOrQuotesTest(String 
recipients) {
+        
when(settings.loadGeneralSettings()).thenReturn(GeneralSettingsVO.builder()
+                .emailRecipients(recipients).build());
+
+        service.dispatch();
+
+        ArgumentCaptor<SimpleMailMessage> message = 
ArgumentCaptor.forClass(SimpleMailMessage.class);
+        verify(mailSender).send(message.capture());
+        
assertThat(message.getValue().getTo()).containsExactly("o...@example.com", 
"bac...@example.com");
+        assertStatus("DELIVERED");
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "o...@example.com (unfinished, bac...@example.com",
+        "o...@example.com (Ops (East, primary), bac...@example.com",
+        "\"unfinished <o...@example.com>, bac...@example.com",
+        "o...@example.com, invalid@@example.com"
+    })
+    void retriesInvalidAddressesWithoutSendingAnyEmailTest(String recipients) {
+        
when(settings.loadGeneralSettings()).thenReturn(GeneralSettingsVO.builder()
+                .emailRecipients(recipients).build());
+
+        service.dispatch();
+
+        verifyNoInteractions(mailSender);
+        assertStatus("RETRY_WAIT");
+    }
+
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    private void assertStatus(String expected) {
+        ArgumentCaptor<UpdateWrapper> update = 
ArgumentCaptor.forClass(UpdateWrapper.class);
+        verify(mapper).update(any(), update.capture());
+        update.getValue().getSqlSet();
+        
assertThat(update.getValue().getParamNameValuePairs().values()).contains(expected);
+    }
+}