opendev/gerrit
Code Issues Proposed changes

Make OutgoingEmailValidator a Singleton

Browse Source 
Make the class a Singleton so that it can be used by classes that get
it injected, rather than calling its methods statically.

This is a preparatory step to allowing the validator to get the gerrit
configuration injected, which will allow to disable validation using
this validator.

Change-Id: If5c7cc6d2c034b9ed16b1358b4d1d186d57daf98
This commit is contained in:
David Pursehouse
2017-01-28 22:03:01 +09:00
parent d2676e215f
commit c7e6021e8b
8 changed files with 46 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java
View File

@@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assert_;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.inject.Inject;
import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -27,9 +28,11 @@ import org.junit.Test;
public class EmailValidatorIT extends AbstractDaemonTest {
private static final String UNSUPPORTED_PREFIX = "#! ";
@Inject private OutgoingEmailValidator validator;
@Test
public void validateLocalDomain() throws Exception {
assertThat(OutgoingEmailValidator.isValid("foo@bar.local")).isTrue();
assertThat(validator.isValid("foo@bar.local")).isTrue();
}
@Test
@@ -49,13 +52,13 @@ public class EmailValidatorIT extends AbstractDaemonTest {
String test = "test@example." + tld.toLowerCase().substring(UNSUPPORTED_PREFIX.length());
assert_()
.withFailureMessage("expected invalid TLD \"" + test + "\"")
.that(OutgoingEmailValidator.isValid(test))
.that(validator.isValid(test))
.isFalse();
} else {
String test = "test@example." + tld.toLowerCase();
assert_()
.withFailureMessage("failed to validate TLD \"" + test + "\"")
.that(OutgoingEmailValidator.isValid(test))
.that(validator.isValid(test))
.isTrue();
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
View File

@@ -77,6 +77,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
private final AuditService auditService;
private final ExternalIds externalIds;
private final ExternalIdsUpdate.User externalIdsUpdateFactory;
private final OutgoingEmailValidator validator;
private final String username;
@Inject
@@ -95,6 +96,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
AuditService auditService,
ExternalIds externalIds,
ExternalIdsUpdate.User externalIdsUpdateFactory,
OutgoingEmailValidator validator,
@Assisted String username) {
this.db = db;
this.currentUser = currentUser;
@@ -110,6 +112,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
this.auditService = auditService;
this.externalIds = externalIds;
this.externalIdsUpdateFactory = externalIdsUpdateFactory;
this.validator = validator;
this.username = username;
}
@@ -141,7 +144,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
if (externalIds.get(db, ExternalId.Key.create(SCHEME_MAILTO, input.email)) != null) {
throw new UnprocessableEntityException("email '" + input.email + "' already exists");
}
if (!OutgoingEmailValidator.isValid(input.email)) {
if (!validator.isValid(input.email)) {
throw new BadRequestException("invalid email address");
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java
View File

@@ -57,6 +57,7 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
private final AccountManager accountManager;
private final RegisterNewEmailSender.Factory registerNewEmailFactory;
private final PutPreferred putPreferred;
private final OutgoingEmailValidator validator;
private final String email;
private final boolean isDevMode;
@@ -69,6 +70,7 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
AccountManager accountManager,
RegisterNewEmailSender.Factory registerNewEmailFactory,
PutPreferred putPreferred,
OutgoingEmailValidator validator,
@Assisted String email) {
this.self = self;
this.realm = realm;
@@ -76,6 +78,7 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
this.accountManager = accountManager;
this.registerNewEmailFactory = registerNewEmailFactory;
this.putPreferred = putPreferred;
this.validator = validator;
this.email = email;
this.isDevMode = authConfig.getAuthType() == DEVELOPMENT_BECOME_ANY_ACCOUNT;
}
@@ -93,7 +96,7 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
input = new EmailInput();
}
if (!OutgoingEmailValidator.isValid(email)) {
if (!validator.isValid(email)) {
throw new BadRequestException("invalid email address");
}

9
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
View File

@@ -44,13 +44,18 @@ public class ExternalIdsConsistencyChecker {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final AccountCache accountCache;
private final OutgoingEmailValidator validator;
@Inject
ExternalIdsConsistencyChecker(
GitRepositoryManager repoManager, AllUsersName allUsers, AccountCache accountCache) {
GitRepositoryManager repoManager,
AllUsersName allUsers,
AccountCache accountCache,
OutgoingEmailValidator validator) {
this.repoManager = repoManager;
this.allUsers = allUsers;
this.accountCache = accountCache;
this.validator = validator;
}
public List<ConsistencyProblemInfo> check() throws IOException {
@@ -123,7 +128,7 @@ public class ExternalIdsConsistencyChecker {
problems);
}
if (extId.email() != null && !OutgoingEmailValidator.isValid(extId.email())) {
if (extId.email() != null && !validator.isValid(extId.email())) {
addError(
String.format(
"External ID '%s' has an invalid email: %s", extId.key().get(), extId.email()),

7
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -101,6 +101,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
private final ProjectCache projectCache;
private final Provider<AnonymousUser> anonymousProvider;
private final PostReviewersOp.Factory postReviewersOpFactory;
private final OutgoingEmailValidator validator;
@Inject
PostReviewers(
@@ -120,7 +121,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
NotifyUtil notifyUtil,
ProjectCache projectCache,
Provider<AnonymousUser> anonymousProvider,
PostReviewersOp.Factory postReviewersOpFactory) {
PostReviewersOp.Factory postReviewersOpFactory,
OutgoingEmailValidator validator) {
this.accounts = accounts;
this.reviewerFactory = reviewerFactory;
this.permissionBackend = permissionBackend;
@@ -138,6 +140,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
this.projectCache = projectCache;
this.anonymousProvider = anonymousProvider;
this.postReviewersOpFactory = postReviewersOpFactory;
this.validator = validator;
}
@Override
@@ -291,7 +294,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
} catch (IllegalArgumentException e) {
throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
}
if (!OutgoingEmailValidator.isValid(adr.getEmail())) {
if (!validator.isValid(adr.getEmail())) {
throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
}
return new Addition(

5
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/EmailArguments.java
View File

@@ -80,6 +80,7 @@ public class EmailArguments {
final DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners;
final StarredChangesUtil starredChangesUtil;
final Provider<InternalAccountQuery> accountQueryProvider;
final OutgoingEmailValidator validator;
@Inject
EmailArguments(
@@ -111,7 +112,8 @@ public class EmailArguments {
SitePaths site,
DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners,
StarredChangesUtil starredChangesUtil,
Provider<InternalAccountQuery> accountQueryProvider) {
Provider<InternalAccountQuery> accountQueryProvider,
OutgoingEmailValidator validator) {
this.server = server;
this.projectCache = projectCache;
this.groupBackend = groupBackend;
@@ -141,5 +143,6 @@ public class EmailArguments {
this.outgoingEmailValidationListeners = outgoingEmailValidationListeners;
this.starredChangesUtil = starredChangesUtil;
this.accountQueryProvider = accountQueryProvider;
this.validator = validator;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
View File

@@ -478,7 +478,7 @@ public abstract class OutgoingEmail {
/** Schedule delivery of this message to the given account. */
protected void add(final RecipientType rt, final Address addr) {
if (addr != null && addr.getEmail() != null && addr.getEmail().length() > 0) {
if (!OutgoingEmailValidator.isValid(addr.getEmail())) {
if (!args.validator.isValid(addr.getEmail())) {
log.warn("Not emailing " + addr.getEmail() + " (invalid email address)");
} else if (!args.emailSender.canEmail(addr.getEmail())) {
log.warn("Not emailing " + addr.getEmail() + " (prohibited by allowrcpt)");

18
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmailValidator.java
View File

@@ -16,15 +16,27 @@ package com.google.gerrit.server.mail.send;
import static org.apache.commons.validator.routines.DomainValidator.ArrayType.GENERIC_PLUS;
import com.google.inject.Singleton;
import org.apache.commons.validator.routines.DomainValidator;
import org.apache.commons.validator.routines.EmailValidator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class OutgoingEmailValidator {
static {
DomainValidator.updateTLDOverride(GENERIC_PLUS, new String[] {"local"});
private static final Logger log = LoggerFactory.getLogger(OutgoingEmailValidator.class);
OutgoingEmailValidator() {
try {
DomainValidator.updateTLDOverride(GENERIC_PLUS, new String[] {"local"});
} catch (IllegalStateException e) {
// Should only happen in tests, where the OutgoingEmailValidator
// is instantiated repeatedly.
log.warn("Failed to update TLD override: " + e.getMessage());
}
}
public static boolean isValid(String addr) {
public boolean isValid(String addr) {
return EmailValidator.getInstance(true, true).isValid(addr);
}
}