chibenwa commented on code in PR #3073:
URL: https://github.com/apache/james-project/pull/3073#discussion_r3494307351
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java:
##########
@@ -62,30 +65,57 @@ public void setSupportsRecipientRewriteTable(boolean
supportsRecipientRewriteTab
this.supportsRecipientRewriteTable = supportsRecipientRewriteTable;
}
+ public void setIgnoreMappingsWithoutLocalMailbox(boolean
ignoreMappingsWithoutLocalMailbox) {
+ this.ignoreMappingsWithoutLocalMailbox =
ignoreMappingsWithoutLocalMailbox;
+ }
+
@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 {
+ LOGGER.debug("Unknown recipient {}, resolving it via RRT", recipient);
try {
- Mappings targetString =
recipientRewriteTable.getResolvedMappings(recipient.getLocalPart(),
recipient.getDomain());
-
- if (!targetString.isEmpty()) {
- return true;
+ // Error mappings are only used to forbid sending from the source
address and can be ignored here.
+ Mappings mappings = recipientRewriteTable.getResolvedMappings(
+ recipient.getLocalPart(),
+ recipient.getDomain(),
+ EnumSet.complementOf(EnumSet.of(Mapping.Type.Error))
+ );
+ if (ignoreMappingsWithoutLocalMailbox) {
+ return !mappings.isEmpty();
+ } else {
+ return allResolvedMailboxesExist(mappings);
}
} catch (ErrorMappingException e) {
- return true;
+ // As we filter the mappings above, this case should never happen.
+ LOGGER.error("Unexpexted mapping of type Error: ", e);
+ return false;
}
- return false;
+ }
+
+ private boolean allResolvedMailboxesExist(Mappings mappings) {
+ return mappings
+ .asStream()
+ .flatMap(mapping -> mapping.asMailAddress().stream())
+ .allMatch(address -> {
Review Comment:
Question: shouldn't this be `any` ?
Eg with the forward feature with a local copy one of the resolved address
would not be local.
--
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]
