You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2020/12/30 03:35:25 UTC

[james-project] 13/29: JAMES-3483 ValidRcptHandler: improve error handling

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit e1ff3bdbe984d0a0466eccdad314dd2887886b15
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sat Dec 26 08:58:57 2020 +0700

    JAMES-3483 ValidRcptHandler: improve error handling
    
    Deny soft upon unexpected exception. That way the sender will
    ultimately resume the transaction.
    
    We discovered that LDAP errors could lead to email loss, as the
    recipient was aggressively rejected in the face of errors.
---
 .../core/fastfail/AbstractValidRcptHandler.java    | 25 +++++++++++------
 .../smtpserver/fastfail/ValidRcptHandler.java      | 31 +++++++---------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/AbstractValidRcptHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/AbstractValidRcptHandler.java
index 23eb4fb..703a499 100644
--- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/AbstractValidRcptHandler.java
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/AbstractValidRcptHandler.java
@@ -39,13 +39,22 @@ public abstract class AbstractValidRcptHandler implements RcptHook {
 
     @Override
     public HookResult doRcpt(SMTPSession session, MaybeSender sender, MailAddress rcpt) {
-        if (!isLocalDomain(session, rcpt.getDomain())) {
+        try {
+            if (!isLocalDomain(session, rcpt.getDomain())) {
+                return HookResult.DECLINED;
+            }
+            if (!isValidRecipient(session, rcpt)) {
+                return reject(rcpt);
+            }
             return HookResult.DECLINED;
+        } catch (Exception e) {
+            LOGGER.error("Encounter an error upon RCPT validation ({}), deny-soft", rcpt.asString(), e);
+            return HookResult.builder()
+                .hookReturnCode(HookReturnCode.denySoft())
+                .smtpReturnCode(SMTPRetCode.LOCAL_ERROR)
+                .smtpDescription(DSNStatus.getStatus(DSNStatus.TRANSIENT, DSNStatus.UNDEFINED_STATUS) + " Unexpected error for " + rcpt.asString())
+                .build();
         }
-        if (!isValidRecipient(session, rcpt)) {
-            return reject(rcpt);
-        }
-        return HookResult.DECLINED;
     }
 
     public HookResult reject(MailAddress rcpt) {
@@ -53,17 +62,17 @@ public abstract class AbstractValidRcptHandler implements RcptHook {
         return HookResult.builder()
             .hookReturnCode(HookReturnCode.deny())
             .smtpReturnCode(SMTPRetCode.MAILBOX_PERM_UNAVAILABLE)
-            .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT,DSNStatus.ADDRESS_MAILBOX) + " Unknown user: " + rcpt.asString())
+            .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.ADDRESS_MAILBOX) + " Unknown user: " + rcpt.asString())
             .build();
     }
 
     /**
      * Return true if email for the given recipient should get accepted
      */
-    protected abstract boolean isValidRecipient(SMTPSession session, MailAddress recipient);
+    protected abstract boolean isValidRecipient(SMTPSession session, MailAddress recipient) throws Exception;
     
     /**
      * Return true if the domain is local
      */
-    protected abstract boolean isLocalDomain(SMTPSession session, Domain domain);
+    protected abstract boolean isLocalDomain(SMTPSession session, Domain domain) throws Exception;
 }
diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java
index 6ca60ed..fe96d4f 100644
--- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java
+++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java
@@ -63,22 +63,17 @@ public class ValidRcptHandler extends AbstractValidRcptHandler implements Protoc
     }
 
     @Override
-    protected boolean isValidRecipient(SMTPSession session, MailAddress recipient) {
-        try {
-            Username username = users.getUsername(recipient);
+    protected boolean isValidRecipient(SMTPSession session, MailAddress recipient) throws UsersRepositoryException, RecipientRewriteTableException {
+        Username username = users.getUsername(recipient);
 
-            if (users.contains(username)) {
-                return true;
-            } else {
-                return supportsRecipientRewriteTable && isRedirected(recipient, username.asString());
-            }
-        } catch (UsersRepositoryException e) {
-            LOGGER.error("Unable to access UsersRepository", e);
-            return false;
+        if (users.contains(username)) {
+            return true;
+        } else {
+            return supportsRecipientRewriteTable && isRedirected(recipient, username.asString());
         }
     }
 
-    private boolean isRedirected(MailAddress recipient, String username) {
+    private boolean isRedirected(MailAddress recipient, String username) throws RecipientRewriteTableException {
         LOGGER.debug("Unknown user {} check if it's an alias", username);
 
         try {
@@ -89,21 +84,13 @@ public class ValidRcptHandler extends AbstractValidRcptHandler implements Protoc
             }
         } catch (ErrorMappingException e) {
             return true;
-        } catch (RecipientRewriteTableException e) {
-            LOGGER.error("Unable to access RecipientRewriteTable", e);
-            return false;
         }
         return false;
     }
 
     @Override
-    protected boolean isLocalDomain(SMTPSession session, Domain domain) {
-        try {
-            return domains.containsDomain(domain);
-        } catch (DomainListException e) {
-            LOGGER.error("Unable to get domains", e);
-            return false;
-        }
+    protected boolean isLocalDomain(SMTPSession session, Domain domain) throws DomainListException {
+        return domains.containsDomain(domain);
     }
 
     @Override


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org