You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/05/14 02:04:44 UTC

[james-project] 09/09: JAMES-3574 LMTP MailetContainerHandler should return one response per RCPT

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

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

commit 1ab3903cb3578e11f9d8f2a0538c2b9744f53ce2
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu May 13 12:37:48 2021 +0700

    JAMES-3574 LMTP MailetContainerHandler should return one response per RCPT
    
    This hook was not spec compliant...
    
    https://datatracker.ietf.org/doc/html/rfc2033#section-4.2
    
    ```
       [...] the server returns one reply
       for each previously successful RCPT command in the mail transaction,
       in the order that the RCPT commands were issued.  Even if there were
       multiple successful RCPT commands giving the same forward-path, there
       must be one reply for each successful RCPT command.
    
    ```
---
 .../james/protocols/lmtp/LMTPMultiResponse.java    |  9 +++-
 .../james/lmtpserver/MailetContainerHandler.java   | 37 +++++++++++---
 .../apache/james/lmtpserver/LmtpServerTest.java    | 56 ++++++++++++++++++++++
 3 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/LMTPMultiResponse.java b/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/LMTPMultiResponse.java
index af212c9..b4b08e4 100644
--- a/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/LMTPMultiResponse.java
+++ b/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/LMTPMultiResponse.java
@@ -29,13 +29,20 @@ import org.apache.james.protocols.api.Response;
  * every recipient. This special {@link Response} can be used for this
  */
 public class LMTPMultiResponse implements Response {
+    public static LMTPMultiResponse of(List<Response> responses) {
+        return new LMTPMultiResponse(responses);
+    }
 
     private final List<Response> responses = new ArrayList<>();
 
+    private LMTPMultiResponse(List<Response> responses) {
+        this.responses.addAll(responses);
+    }
+
     public LMTPMultiResponse(Response response) {
         addResponse(response);
     }
-    
+
     public void addResponse(Response response) {
         this.responses.add(response);
         
diff --git a/server/protocols/protocols-lmtp/src/main/java/org/apache/james/lmtpserver/MailetContainerHandler.java b/server/protocols/protocols-lmtp/src/main/java/org/apache/james/lmtpserver/MailetContainerHandler.java
index 628f88e..2ca8d48 100644
--- a/server/protocols/protocols-lmtp/src/main/java/org/apache/james/lmtpserver/MailetContainerHandler.java
+++ b/server/protocols/protocols-lmtp/src/main/java/org/apache/james/lmtpserver/MailetContainerHandler.java
@@ -19,10 +19,14 @@
 
 package org.apache.james.lmtpserver;
 
+import java.util.Collection;
+
 import javax.inject.Inject;
 
+import org.apache.james.core.MailAddress;
 import org.apache.james.mailetcontainer.api.MailProcessor;
 import org.apache.james.protocols.api.Response;
+import org.apache.james.protocols.lmtp.LMTPMultiResponse;
 import org.apache.james.protocols.smtp.SMTPResponse;
 import org.apache.james.protocols.smtp.SMTPRetCode;
 import org.apache.james.protocols.smtp.SMTPSession;
@@ -33,6 +37,9 @@ import org.apache.james.protocols.smtp.hook.HookReturnCode;
 import org.apache.james.smtpserver.DataLineJamesMessageHookHandler;
 import org.apache.mailet.Mail;
 
+import com.github.steveash.guavate.Guavate;
+import com.google.common.collect.ImmutableList;
+
 public class MailetContainerHandler extends DataLineJamesMessageHookHandler {
     private final MailProcessor mailProcessor;
 
@@ -43,17 +50,35 @@ public class MailetContainerHandler extends DataLineJamesMessageHookHandler {
 
     @Override
     protected Response processExtensions(SMTPSession session, Mail mail) {
+        Collection<MailAddress> recipients = ImmutableList.copyOf(mail.getRecipients());
         try {
             super.processExtensions(session, mail);
+
+            if (recipients.size() == 0) {
+                // Return 503 see https://datatracker.ietf.org/doc/html/rfc2033#section-4.2
+                AbstractHookableCmdHandler.calcDefaultSMTPResponse(HookResult.builder()
+                    .hookReturnCode(HookReturnCode.ok())
+                    .smtpReturnCode(SMTPRetCode.MAIL_OK)
+                    .smtpDescription(DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.CONTENT_OTHER) + " Message received")
+                    .build());
+            }
+
             mailProcessor.service(mail);
 
-            return AbstractHookableCmdHandler.calcDefaultSMTPResponse(HookResult.builder()
-                .hookReturnCode(HookReturnCode.ok())
-                .smtpReturnCode(SMTPRetCode.MAIL_OK)
-                .smtpDescription(DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.CONTENT_OTHER) + " Message received")
-                .build());
+            return LMTPMultiResponse.of(
+                recipients.stream()
+                    .map(recipient -> AbstractHookableCmdHandler.calcDefaultSMTPResponse(HookResult.builder()
+                        .hookReturnCode(HookReturnCode.ok())
+                        .smtpReturnCode(SMTPRetCode.MAIL_OK)
+                        .smtpDescription(DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.CONTENT_OTHER) + " Message received <" + recipient.asString() + ">")
+                        .build()))
+                    .collect(Guavate.toImmutableList()));
+
         } catch (Exception e) {
-            return new SMTPResponse(SMTPRetCode.LOCAL_ERROR, DSNStatus.getStatus(DSNStatus.TRANSIENT, DSNStatus.UNDEFINED_STATUS) + " Temporary error deliver message");
+            return LMTPMultiResponse.of(
+                recipients.stream()
+                    .map(recipient -> new SMTPResponse(SMTPRetCode.LOCAL_ERROR, DSNStatus.getStatus(DSNStatus.TRANSIENT, DSNStatus.UNDEFINED_STATUS) + " Temporary error deliver message <" + recipient.asString() + ">"))
+                    .collect(Guavate.toImmutableList()));
         }
     }
 }
diff --git a/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java b/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
index 3184e5d..3545575 100644
--- a/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
+++ b/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
@@ -201,6 +201,34 @@ class LmtpServerTest {
 
             assertThat(recordingMailProcessor.getMails()).hasSize(1);
         }
+
+        @Test
+        void dataShouldHaveAReturnCodePerRecipient() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, getLmtpPort(lmtpServerFactory)));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(("LHLO <" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("MAIL FROM: <bob@" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("RCPT TO: <bo...@examplebis.local>\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("RCPT TO: <ce...@examplebis.local>\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server); // needed to synchronize
+            server.write(ByteBuffer.wrap(("header:value\r\n\r\nbody").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8)));
+            byte[] dataResponse = readBytes(server);
+            server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(new String(dataResponse, StandardCharsets.UTF_8))
+                .contains("250 2.6.0 Message received <bo...@examplebis.local>\r\n" +
+                    "250 2.6.0 Message received <ce...@examplebis.local>");
+        }
     }
 
     @Nested
@@ -293,6 +321,34 @@ class LmtpServerTest {
             assertThat(new String(dataResponse, StandardCharsets.UTF_8))
                 .startsWith("451 4.0.0 Temporary error deliver message");
         }
+
+        @Test
+        void dataShouldHaveAReturnCodePerRecipient() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, getLmtpPort(lmtpServerFactory)));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(("LHLO <" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("MAIL FROM: <bob@" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("RCPT TO: <bo...@examplebis.local>\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("RCPT TO: <ce...@examplebis.local>\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8)));
+            readBytes(server); // needed to synchronize
+            server.write(ByteBuffer.wrap(("header:value\r\n\r\nbody").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8)));
+            server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8)));
+            byte[] dataResponse = readBytes(server);
+            server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(new String(dataResponse, StandardCharsets.UTF_8))
+                .contains("451 4.0.0 Temporary error deliver message <bo...@examplebis.local>\r\n" +
+                    "451 4.0.0 Temporary error deliver message <ce...@examplebis.local>");
+        }
     }
 
     @Nested

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