Watcher: Avoid NPE when local address is not resolvable (elastic/elasticsearch#3910)

This prevents a possible NPE when sending emails, as some host have
a perfectly fine internet connection, but cannot resolve their localhost.

In addition I also removed a EmailService.send() method that was only used
in tests and thus not needed.

Closes elastic/elasticsearch#3227

Original commit: elastic/x-pack-elasticsearch@d2e29b4c92
This commit is contained in:
Alexander Reelsen 2016-11-07 11:55:50 +01:00 committed by GitHub
parent 8b6552516e
commit ecb5bc89dc
8 changed files with 30 additions and 57 deletions

View File

@ -89,11 +89,13 @@ public class Account {
Transport transport = session.getTransport(SMTP_PROTOCOL);
String user = auth != null ? auth.user() : null;
String user = auth != null ? auth.user() : config.smtp.user;
if (user == null) {
user = config.smtp.user;
if (user == null) {
user = InternetAddress.getLocalAddress(session).getAddress();
InternetAddress localAddress = InternetAddress.getLocalAddress(session);
// null check needed, because if the local host does not resolve, this may be null
// this can happen in wrongly setup linux distributions
if (localAddress != null) {
user = localAddress.getAddress();
}
}

View File

@ -37,10 +37,6 @@ public class EmailService extends NotificationService<Account> {
}
public EmailSent send(Email email, Authentication auth, Profile profile) throws MessagingException {
return send(email, auth, profile, (String) null);
}
public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) throws MessagingException {
Account account = getAccount(accountName);
if (account == null) {
@ -50,7 +46,7 @@ public class EmailService extends NotificationService<Account> {
return send(email, auth, profile, account);
}
EmailSent send(Email email, Authentication auth, Profile profile, Account account) throws MessagingException {
private EmailSent send(Email email, Authentication auth, Profile profile, Account account) throws MessagingException {
assert account != null;
try {
email = account.send(email, auth, profile);

View File

@ -9,8 +9,8 @@ import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.notification.email.support.EmailServer;
import org.elasticsearch.xpack.common.secret.Secret;
import org.elasticsearch.xpack.notification.email.support.EmailServer;
import org.junit.After;
import org.junit.Before;

View File

@ -9,7 +9,6 @@ import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.notification.NotificationService;
import java.util.Collections;
@ -85,18 +84,13 @@ public class AccountsTests extends ESTestCase {
}
public void testMultipleAccountsUnknownDefault() throws Exception {
Settings.Builder builder = Settings.builder()
.put("xpack.notification.email.default_account", "unknown");
Settings.Builder builder = Settings.builder().put("xpack.notification.email.default_account", "unknown");
addAccountSettings("account1", builder);
addAccountSettings("account2", builder);
try {
new EmailService(builder.build(), null,
new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING)));
fail("Expected SettingsException");
} catch (SettingsException e) {
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING));
SettingsException e = expectThrows(SettingsException.class, () -> new EmailService(builder.build(), null, clusterSettings));
assertThat(e.getMessage(), is("could not find default account [unknown]"));
}
}
public void testNoAccount() throws Exception {
Settings.Builder builder = Settings.builder();
@ -106,16 +100,11 @@ public class AccountsTests extends ESTestCase {
}
public void testNoAccountWithDefaultAccount() throws Exception {
Settings.Builder builder = Settings.builder()
.put("xpack.notification.email.default_account", "unknown");
try {
new EmailService(builder.build(), null,
new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING)));
fail("Expected SettingsException");
} catch (SettingsException e) {
Settings settings = Settings.builder().put("xpack.notification.email.default_account", "unknown").build();
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING));
SettingsException e = expectThrows(SettingsException.class, () -> new EmailService(settings, null, clusterSettings));
assertThat(e.getMessage(), is("could not find default account [unknown]"));
}
}
private void addAccountSettings(String name, Settings.Builder builder) {
builder.put("xpack.notification.email.account." + name + ".smtp.host", "_host");

View File

@ -23,7 +23,7 @@ public class ManualPublicSmtpServersTester {
public static class Gmail {
public static void main(String[] args) throws Exception {
test(Profile.GMAIL, Settings.builder()
test("gmail", Profile.GMAIL, Settings.builder()
.put("xpack.notification.email.account.gmail.smtp.auth", true)
.put("xpack.notification.email.account.gmail.smtp.starttls.enable", true)
.put("xpack.notification.email.account.gmail.smtp.host", "smtp.gmail.com")
@ -38,7 +38,7 @@ public class ManualPublicSmtpServersTester {
public static class OutlookDotCom {
public static void main(String[] args) throws Exception {
test(Profile.STANDARD, Settings.builder()
test("outlook", Profile.STANDARD, Settings.builder()
.put("xpack.notification.email.account.outlook.smtp.auth", true)
.put("xpack.notification.email.account.outlook.smtp.starttls.enable", true)
.put("xpack.notification.email.account.outlook.smtp.host", "smtp-mail.outlook.com")
@ -54,7 +54,7 @@ public class ManualPublicSmtpServersTester {
public static class YahooMail {
public static void main(String[] args) throws Exception {
test(Profile.STANDARD, Settings.builder()
test("yahoo", Profile.STANDARD, Settings.builder()
.put("xpack.notification.email.account.yahoo.smtp.starttls.enable", true)
.put("xpack.notification.email.account.yahoo.smtp.auth", true)
.put("xpack.notification.email.account.yahoo.smtp.host", "smtp.mail.yahoo.com")
@ -72,7 +72,7 @@ public class ManualPublicSmtpServersTester {
public static class SES {
public static void main(String[] args) throws Exception {
test(Profile.STANDARD, Settings.builder()
test("ses", Profile.STANDARD, Settings.builder()
.put("xpack.notification.email.account.ses.smtp.auth", true)
.put("xpack.notification.email.account.ses.smtp.starttls.enable", true)
.put("xpack.notification.email.account.ses.smtp.starttls.required", true)
@ -87,7 +87,7 @@ public class ManualPublicSmtpServersTester {
}
}
static void test(Profile profile, Settings.Builder settingsBuilder) throws Exception {
static void test(String accountName, Profile profile, Settings.Builder settingsBuilder) throws Exception {
String path = "/org/elasticsearch/xpack/watcher/actions/email/service/logo.png";
if (EmailServiceTests.class.getResourceAsStream(path) == null) {
throw new ElasticsearchException("Could not find logo at path {}", path);
@ -110,7 +110,7 @@ public class ManualPublicSmtpServersTester {
() -> EmailServiceTests.class.getResourceAsStream(path)))
.build();
EmailService.EmailSent sent = service.send(email, null, profile);
EmailService.EmailSent sent = service.send(email, null, profile, accountName);
terminal.println(String.format(Locale.ROOT, "email sent via account [%s]", sent.account()));
}

View File

@ -15,10 +15,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.common.text.TextTemplateEngine;
import org.elasticsearch.xpack.watcher.actions.Action;
import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.execution.Wid;
import org.elasticsearch.xpack.common.http.HttpClient;
import org.elasticsearch.xpack.common.http.HttpRequest;
import org.elasticsearch.xpack.common.http.HttpRequestTemplate;
@ -27,10 +23,7 @@ import org.elasticsearch.xpack.common.http.auth.HttpAuthRegistry;
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuthFactory;
import org.elasticsearch.xpack.common.secret.Secret;
import org.elasticsearch.xpack.common.text.TextTemplate;
import org.elasticsearch.xpack.watcher.support.xcontent.WatcherParams;
import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase;
import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine;
import org.elasticsearch.xpack.watcher.watch.Payload;
import org.elasticsearch.xpack.common.text.TextTemplateEngine;
import org.elasticsearch.xpack.notification.email.Attachment;
import org.elasticsearch.xpack.notification.email.Authentication;
import org.elasticsearch.xpack.notification.email.DataAttachment;
@ -45,6 +38,13 @@ import org.elasticsearch.xpack.notification.email.attachment.EmailAttachments;
import org.elasticsearch.xpack.notification.email.attachment.EmailAttachmentsParser;
import org.elasticsearch.xpack.notification.email.attachment.HttpEmailAttachementParser;
import org.elasticsearch.xpack.notification.email.attachment.HttpRequestAttachment;
import org.elasticsearch.xpack.watcher.actions.Action;
import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.execution.Wid;
import org.elasticsearch.xpack.watcher.support.xcontent.WatcherParams;
import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase;
import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine;
import org.elasticsearch.xpack.watcher.watch.Payload;
import org.jboss.netty.handler.codec.http.HttpHeaders;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
@ -94,11 +94,6 @@ public class EmailActionTests extends ESTestCase {
public void testExecute() throws Exception {
final String account = "account1";
EmailService service = new AbstractWatcherIntegrationTestCase.NoopEmailService() {
@Override
public EmailService.EmailSent send(Email email, Authentication auth, Profile profile) {
return new EmailService.EmailSent(account, email);
}
@Override
public EmailService.EmailSent send(Email email, Authentication auth, Profile profile, String accountName) {
return new EmailService.EmailSent(account, email);

View File

@ -274,10 +274,6 @@ public class WebhookActionTests extends ESTestCase {
mock(WatcherClientProxy.class),
ExecuteScenario.Success.client(),
new AbstractWatcherIntegrationTestCase.NoopEmailService() {
@Override
public EmailSent send(Email email, Authentication auth, Profile profile) {
return new EmailSent(account, email);
}
@Override
public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) {

View File

@ -662,11 +662,6 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase
new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING)));
}
@Override
public EmailService.EmailSent send(Email email, Authentication auth, Profile profile) {
return new EmailSent(auth.user(), email);
}
@Override
public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) {
return new EmailSent(accountName, email);