浏览代码

Ensure watcher email action message ids are always unique (#56574)

If an email action is used in a foreach loop, message ids could have
been duplicated, which then get rejected by the mail server.

This commit introduces an additional static counter in the email action
in order to ensure that every message id is unique.
Alexander Reelsen 5 年之前
父节点
当前提交
9c9075cb8e

+ 7 - 1
x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java

@@ -25,9 +25,12 @@ import org.elasticsearch.xpack.watcher.support.Variables;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicLong;
 
 public class ExecutableEmailAction extends ExecutableAction<EmailAction> {
 
+    private static final AtomicLong counter = new AtomicLong(0);
+
     private final EmailService emailService;
     private final TextTemplateEngine templateEngine;
     private final HtmlSanitizer htmlSanitizer;
@@ -67,7 +70,10 @@ public class ExecutableEmailAction extends ExecutableAction<EmailAction> {
         }
 
         Email.Builder email = action.getEmail().render(templateEngine, model, htmlSanitizer, attachments);
-        email.id(actionId + "_" + ctx.id().value());
+        // the counter ensures that a different message id is generated, even if the method is called with the same parameters
+        // this may happen if a foreach loop is used for this action
+        // same message ids will result in emails not being accepted by mail servers and thus have to be prevented at all times
+        email.id(actionId + "_" + ctx.id().value() + "_" + Long.toUnsignedString(counter.incrementAndGet()));
 
         if (ctx.simulateAction(actionId)) {
             return new EmailAction.Result.Simulated(email.build());

+ 8 - 1
x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java

@@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.startsWith;
@@ -170,7 +171,7 @@ public class EmailActionTests extends ESTestCase {
         assertThat(result, instanceOf(EmailAction.Result.Success.class));
         assertThat(((EmailAction.Result.Success) result).account(), equalTo(account));
         Email actualEmail = ((EmailAction.Result.Success) result).email();
-        assertThat(actualEmail.id(), is("_id_" + wid.value()));
+        assertThat(actualEmail.id(), startsWith("_id_" + wid.value() + "_"));
         assertThat(actualEmail, notNullValue());
         assertThat(actualEmail.subject(), is(subject == null ? null : subject.getTemplate()));
         assertThat(actualEmail.textBody(), is(textBody == null ? null : textBody.getTemplate()));
@@ -178,6 +179,12 @@ public class EmailActionTests extends ESTestCase {
         if (dataAttachment != null) {
             assertThat(actualEmail.attachments(), hasKey("data"));
         }
+
+        // a second execution with the same parameters may not yield the same message id
+        result = executable.execute("_id", ctx, payload);
+        String oldMessageId = actualEmail.id();
+        String newMessageId = ((EmailAction.Result.Success) result).email().id();
+        assertThat(oldMessageId, is(not(newMessageId)));
     }
 
     public void testParser() throws Exception {