Arsnael commented on code in PR #2536:
URL: https://github.com/apache/james-project/pull/2536#discussion_r1866972704


##########
protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/core/ReceivedDataLineFilter.java:
##########
@@ -19,19 +19,45 @@
 package org.apache.james.protocols.lmtp.core;
 
 import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.core.ReceivedHeaderGenerator;
 
 /**
  * {@link ReceivedDataLineFilter} which will add the Received header to the 
message
  */
 public class ReceivedDataLineFilter extends 
org.apache.james.protocols.smtp.core.ReceivedDataLineFilter {
 
     private static final String SERVICE_TYPE = "LMTP";
-    
-    /**
-     * Always returns <code>LMTP</code>
-     */
-    @Override
-    protected String getServiceType(SMTPSession session, String heloMode) {
-        return SERVICE_TYPE;
+    private static final String SERVICE_TYPE_AUTH = "LMTPA";
+    private static final String SERVICE_TYPE_SSL = "LMTPS";
+    private static final String SERVICE_TYPE_SSL_AUTH = "LMTPSA";
+
+    public ReceivedDataLineFilter() {
+        super(new ReceivedHeaderGenerator() {
+
+            /**
+             * Always returns <code>LMTP</code>
+             */
+            @Override
+            protected String getServiceType(SMTPSession session, String 
heloMode) {
+                // Not successful auth
+                if (session.getUsername() == null) {
+                    if (session.isTLSStarted()) {
+                        return SERVICE_TYPE_SSL;
+                    } else {
+                        return SERVICE_TYPE;
+                    }
+                } else {
+                    // See RFC3848:
+                    // The new keyword "ESMTPA" indicates the use of ESMTP 
when the SMTP
+                    // AUTH [3] extension is also used and authentication is 
successfully achieved.
+                    if (session.isTLSStarted()) {
+                        return SERVICE_TYPE_SSL_AUTH;
+                    } else {
+                        return SERVICE_TYPE_SSL;

Review Comment:
   shouldn't we return here SERVICE_TYPE_AUTH ?



##########
protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedHeaderGenerator.java:
##########
@@ -0,0 +1,126 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.protocols.smtp.core;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.List;
+import java.util.Locale;
+import java.util.Optional;
+
+import org.apache.james.core.MailAddress;
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPSession;
+
+import com.google.common.collect.ImmutableList;
+
+public class ReceivedHeaderGenerator {
+    private static final DateTimeFormatter DATEFORMAT = 
DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss Z (zzz)", Locale.US);
+
+    private static final String EHLO = "EHLO";
+    private static final String SMTP = "SMTP";
+    private static final String ESMTPA = "ESMTPA";
+    private static final String ESMTPSA = "ESMTPA";
+    private static final String ESMTP = "ESMTP";
+    private static final String ESMTPS = "ESMTP";

Review Comment:
   Any reason that `ESMTPSA` constant is "ESMTPA" and not "ESMTPSA"?
   
   Same question for `ESMTPS`



-- 
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: notifications-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to