chibenwa commented on code in PR #3073:
URL: https://github.com/apache/james-project/pull/3073#discussion_r3585031274
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java:
##########
@@ -62,32 +81,92 @@ public void setSupportsRecipientRewriteTable(boolean
supportsRecipientRewriteTab
this.supportsRecipientRewriteTable = supportsRecipientRewriteTable;
}
+ public void setRecipientRewriteTableCheck(RecipientRewriteTableCheck
recipientRewriteTableCheck) {
+ this.recipientRewriteTableCheck = recipientRewriteTableCheck;
+ }
+
@Override
protected boolean isValidRecipient(SMTPSession session, MailAddress
recipient) throws UsersRepositoryException, RecipientRewriteTableException {
- Username username = users.getUsername(recipient);
-
- if (users.contains(username)) {
+ // Check existence of mailbox first to use RRT less often.
+ if (mailboxExists(recipient)) {
return true;
} else {
- return supportsRecipientRewriteTable && isRedirected(recipient,
username.asString());
+ // Check whether there is a valid RRT entry for the recipient.
+ return supportsRecipientRewriteTable &&
hasValidRRTEntry(recipient);
}
}
- private boolean isRedirected(MailAddress recipient, String username)
throws RecipientRewriteTableException {
- LOGGER.debug("Unknown user {} check if it's an alias", username);
+ protected boolean mailboxExists(MailAddress recipient) throws
UsersRepositoryException {
+ return users.contains(users.getUsername(recipient));
+ }
+
+ protected boolean hasValidRRTEntry(MailAddress recipient) throws
RecipientRewriteTableException, UsersRepositoryException {
+ LOGGER.debug("Unknown recipient {}, resolving it via RRT", recipient);
try {
- Mappings targetString =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield !mappings.isEmpty();
+ }
+ case ANY_TARGET_HAS_LOCAL_MAILBOX -> {
+ // As long as there is any mapping to a local mailbox,
this check passes.
+ // Error mappings are therefore irrelevant.
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(
+ recipient.getLocalPart(),
+ recipient.getDomain(),
+ EnumSet.complementOf(EnumSet.of(Mapping.Type.Error))
+ );
+ yield anyResolvedMailboxExists(mappings);
+ }
+ case ALL_TARGETS_HAVE_LOCAL_MAILBOX -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield allResolvedMailboxesExist(mappings);
+ }
+ };
+ } catch (ErrorMappingException e) {
+ // Either an error mapping was encountered (ErrorMappingException)
or the limit for recursively
+ // resolving a mapping was reached (TooManyMappingException).
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ // An error during mapping means that a mapping exists.
+ LOGGER.debug("Error while resolving recipient via RRT,
allowing recipient {}: ", recipient, e);
Review Comment:
```suggestion
LOGGER.info("Error while resolving recipient via RRT,
allowing recipient {}: ", recipient, e);
```
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java:
##########
@@ -62,32 +81,92 @@ public void setSupportsRecipientRewriteTable(boolean
supportsRecipientRewriteTab
this.supportsRecipientRewriteTable = supportsRecipientRewriteTable;
}
+ public void setRecipientRewriteTableCheck(RecipientRewriteTableCheck
recipientRewriteTableCheck) {
+ this.recipientRewriteTableCheck = recipientRewriteTableCheck;
+ }
+
@Override
protected boolean isValidRecipient(SMTPSession session, MailAddress
recipient) throws UsersRepositoryException, RecipientRewriteTableException {
- Username username = users.getUsername(recipient);
-
- if (users.contains(username)) {
+ // Check existence of mailbox first to use RRT less often.
+ if (mailboxExists(recipient)) {
return true;
} else {
- return supportsRecipientRewriteTable && isRedirected(recipient,
username.asString());
+ // Check whether there is a valid RRT entry for the recipient.
+ return supportsRecipientRewriteTable &&
hasValidRRTEntry(recipient);
}
}
- private boolean isRedirected(MailAddress recipient, String username)
throws RecipientRewriteTableException {
- LOGGER.debug("Unknown user {} check if it's an alias", username);
+ protected boolean mailboxExists(MailAddress recipient) throws
UsersRepositoryException {
+ return users.contains(users.getUsername(recipient));
+ }
+
+ protected boolean hasValidRRTEntry(MailAddress recipient) throws
RecipientRewriteTableException, UsersRepositoryException {
+ LOGGER.debug("Unknown recipient {}, resolving it via RRT", recipient);
try {
- Mappings targetString =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield !mappings.isEmpty();
+ }
+ case ANY_TARGET_HAS_LOCAL_MAILBOX -> {
+ // As long as there is any mapping to a local mailbox,
this check passes.
+ // Error mappings are therefore irrelevant.
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(
+ recipient.getLocalPart(),
+ recipient.getDomain(),
+ EnumSet.complementOf(EnumSet.of(Mapping.Type.Error))
+ );
+ yield anyResolvedMailboxExists(mappings);
+ }
+ case ALL_TARGETS_HAVE_LOCAL_MAILBOX -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield allResolvedMailboxesExist(mappings);
+ }
+ };
+ } catch (ErrorMappingException e) {
+ // Either an error mapping was encountered (ErrorMappingException)
or the limit for recursively
+ // resolving a mapping was reached (TooManyMappingException).
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ // An error during mapping means that a mapping exists.
+ LOGGER.debug("Error while resolving recipient via RRT,
allowing recipient {}: ", recipient, e);
+ yield true;
+ }
+ case ANY_TARGET_HAS_LOCAL_MAILBOX -> {
+ // It is unclear whether at least one mapping is valid.
+ LOGGER.debug("Error while resolving recipient via RRT,
refusing recipient {}: ", recipient, e);
Review Comment:
```suggestion
LOGGER.info("Error while resolving recipient via RRT,
refusing recipient {}: ", recipient, e);
```
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java:
##########
@@ -62,32 +81,92 @@ public void setSupportsRecipientRewriteTable(boolean
supportsRecipientRewriteTab
this.supportsRecipientRewriteTable = supportsRecipientRewriteTable;
}
+ public void setRecipientRewriteTableCheck(RecipientRewriteTableCheck
recipientRewriteTableCheck) {
+ this.recipientRewriteTableCheck = recipientRewriteTableCheck;
+ }
+
@Override
protected boolean isValidRecipient(SMTPSession session, MailAddress
recipient) throws UsersRepositoryException, RecipientRewriteTableException {
- Username username = users.getUsername(recipient);
-
- if (users.contains(username)) {
+ // Check existence of mailbox first to use RRT less often.
+ if (mailboxExists(recipient)) {
return true;
} else {
- return supportsRecipientRewriteTable && isRedirected(recipient,
username.asString());
+ // Check whether there is a valid RRT entry for the recipient.
+ return supportsRecipientRewriteTable &&
hasValidRRTEntry(recipient);
}
}
- private boolean isRedirected(MailAddress recipient, String username)
throws RecipientRewriteTableException {
- LOGGER.debug("Unknown user {} check if it's an alias", username);
+ protected boolean mailboxExists(MailAddress recipient) throws
UsersRepositoryException {
+ return users.contains(users.getUsername(recipient));
+ }
+
+ protected boolean hasValidRRTEntry(MailAddress recipient) throws
RecipientRewriteTableException, UsersRepositoryException {
+ LOGGER.debug("Unknown recipient {}, resolving it via RRT", recipient);
try {
- Mappings targetString =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield !mappings.isEmpty();
+ }
+ case ANY_TARGET_HAS_LOCAL_MAILBOX -> {
+ // As long as there is any mapping to a local mailbox,
this check passes.
+ // Error mappings are therefore irrelevant.
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(
+ recipient.getLocalPart(),
+ recipient.getDomain(),
+ EnumSet.complementOf(EnumSet.of(Mapping.Type.Error))
+ );
+ yield anyResolvedMailboxExists(mappings);
+ }
+ case ALL_TARGETS_HAVE_LOCAL_MAILBOX -> {
+ Mappings mappings =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
+ yield allResolvedMailboxesExist(mappings);
+ }
+ };
+ } catch (ErrorMappingException e) {
+ // Either an error mapping was encountered (ErrorMappingException)
or the limit for recursively
+ // resolving a mapping was reached (TooManyMappingException).
+ return switch (this.recipientRewriteTableCheck) {
+ case MAPPING_EXISTS -> {
+ // An error during mapping means that a mapping exists.
+ LOGGER.debug("Error while resolving recipient via RRT,
allowing recipient {}: ", recipient, e);
+ yield true;
+ }
+ case ANY_TARGET_HAS_LOCAL_MAILBOX -> {
+ // It is unclear whether at least one mapping is valid.
+ LOGGER.debug("Error while resolving recipient via RRT,
refusing recipient {}: ", recipient, e);
+ yield false;
+ }
+ case ALL_TARGETS_HAVE_LOCAL_MAILBOX -> {
+ // It is unclear whether all mappings are valid.
+ LOGGER.debug("Error while resolving recipient via RRT,
refusing recipient {}: ", recipient, e);
Review Comment:
```suggestion
LOGGER.info("Error while resolving recipient via RRT,
refusing recipient {}: ", recipient, e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
