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.
This commit is contained in:
Alexander Reelsen 2020-05-14 10:19:30 +02:00
parent 4438115be0
commit 3a263d91f6
2 changed files with 15 additions and 2 deletions

View File

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

View File

@ -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 {