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/07/21 06:48:17 UTC

[james-project] 01/02: JAMES-3613 SMTP ChannelUpstreamHandler should compute transport MDC upon connection

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 fc28d3b2b1a2324ed081b66bfcacb1b77eb30c1c
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Jul 14 17:07:11 2021 +0700

    JAMES-3613 SMTP ChannelUpstreamHandler should compute transport MDC upon connection
    
    Later interactions should be able to reuse the initial MDC.
    
    This, for instance, avoid computing the MDC for each following message, but also
    prevents triggering several DNS queries related to the same transport connection.
    
    We do save the initial MDC as a session attachment for later reuse.
    
    Note that subsequent message might update the state of the SMTP connection, eg
    by doing authentication thus we need to refresh the MDC context linked to the
    protocol session on each message.
---
 .../netty/BasicChannelUpstreamHandler.java         | 30 +++++++++++++++++-----
 .../protocols/netty/ProtocolMDCContextFactory.java | 17 +++++++-----
 .../protocols/smtp/core/SMTPMDCContextFactory.java | 18 ++++++-------
 .../java/org/apache/james/util/MDCBuilder.java     |  5 ++++
 4 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/protocols/netty/src/main/java/org/apache/james/protocols/netty/BasicChannelUpstreamHandler.java b/protocols/netty/src/main/java/org/apache/james/protocols/netty/BasicChannelUpstreamHandler.java
index 0adcb0c..9732306 100644
--- a/protocols/netty/src/main/java/org/apache/james/protocols/netty/BasicChannelUpstreamHandler.java
+++ b/protocols/netty/src/main/java/org/apache/james/protocols/netty/BasicChannelUpstreamHandler.java
@@ -18,10 +18,13 @@
  ****************************************************************/
 package org.apache.james.protocols.netty;
 
+import static org.apache.james.protocols.api.ProtocolSession.State.Connection;
+
 import java.io.Closeable;
 import java.nio.channels.ClosedChannelException;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
 
 import javax.net.ssl.SSLEngine;
 
@@ -36,6 +39,7 @@ import org.apache.james.protocols.api.handler.DisconnectHandler;
 import org.apache.james.protocols.api.handler.LineHandler;
 import org.apache.james.protocols.api.handler.ProtocolHandlerChain;
 import org.apache.james.protocols.api.handler.ProtocolHandlerResultHandler;
+import org.apache.james.util.MDCBuilder;
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.channel.Channel;
 import org.jboss.netty.channel.ChannelHandler.Sharable;
@@ -55,6 +59,7 @@ import org.slf4j.LoggerFactory;
 @Sharable
 public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
     private static final Logger LOGGER = LoggerFactory.getLogger(BasicChannelUpstreamHandler.class);
+    private static final ProtocolSession.AttachmentKey<MDCBuilder> MDC_KEY = ProtocolSession.AttachmentKey.of("bound_MDC", MDCBuilder.class);
 
     private final ProtocolMDCContextFactory mdcContextFactory;
     protected final Protocol protocol;
@@ -75,13 +80,24 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
 
     @Override
     public void channelBound(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
-            ctx.setAttachment(createSession(ctx));
+        MDCBuilder boundMDC = mdcContextFactory.onBound(protocol, ctx);
+        try (Closeable closeable = boundMDC.build()) {
+            ProtocolSession session = createSession(ctx);
+            session.setAttachment(MDC_KEY, boundMDC, Connection);
+            ctx.setAttachment(session);
             super.channelBound(ctx, e);
         }
     }
 
+    private MDCBuilder mdc(ChannelHandlerContext ctx) {
+        ProtocolSession session = (ProtocolSession) ctx.getAttachment();
 
+        return Optional.ofNullable(session)
+            .flatMap(s -> s.getAttachment(MDC_KEY, Connection))
+            .map(mdc -> mdcContextFactory.withContext(session)
+                .addToContext(mdc))
+            .orElseGet(MDCBuilder::create);
+    }
 
     /**
      * Call the {@link ConnectHandler} instances which are stored in the {@link ProtocolHandlerChain}
@@ -89,7 +105,7 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
     @SuppressWarnings({ "unchecked", "rawtypes" })
     @Override
     public void channelConnected(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
+        try (Closeable closeable = mdc(ctx).build()) {
             List<ConnectHandler> connectHandlers = chain.getHandlers(ConnectHandler.class);
             List<ProtocolHandlerResultHandler> resultHandlers = chain.getHandlers(ProtocolHandlerResultHandler.class);
             ProtocolSession session = (ProtocolSession) ctx.getAttachment();
@@ -119,7 +135,7 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
     @SuppressWarnings({ "rawtypes", "unchecked" })
     @Override
     public void channelDisconnected(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
+        try (Closeable closeable = mdc(ctx).build()) {
             List<DisconnectHandler> connectHandlers = chain.getHandlers(DisconnectHandler.class);
             ProtocolSession session = (ProtocolSession) ctx.getAttachment();
             if (connectHandlers != null) {
@@ -138,7 +154,7 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
     @SuppressWarnings({ "unchecked", "rawtypes" })
     @Override
     public void messageReceived(ChannelHandlerContext ctx, MessageEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
+        try (Closeable closeable = mdc(ctx).build()) {
             ProtocolSession pSession = (ProtocolSession) ctx.getAttachment();
             LinkedList<LineHandler> lineHandlers = chain.getHandlers(LineHandler.class);
             LinkedList<ProtocolHandlerResultHandler> resultHandlers = chain.getHandlers(ProtocolHandlerResultHandler.class);
@@ -169,7 +185,7 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
 
     @Override
     public void channelClosed(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
+        try (Closeable closeable = mdc(ctx).build()) {
             ProtocolSession session = (ProtocolSession) ctx.getAttachment();
             LOGGER.info("Connection closed for {}", session.getRemoteAddress().getAddress().getHostAddress());
             cleanup(ctx);
@@ -206,7 +222,7 @@ public class BasicChannelUpstreamHandler extends SimpleChannelUpstreamHandler {
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e) throws Exception {
-        try (Closeable closeable = mdcContextFactory.from(protocol, ctx)) {
+        try (Closeable closeable = mdc(ctx).build()) {
             Channel channel = ctx.getChannel();
             ProtocolSession session = (ProtocolSession) ctx.getAttachment();
             if (e.getCause() instanceof TooLongFrameException && session != null) {
diff --git a/protocols/netty/src/main/java/org/apache/james/protocols/netty/ProtocolMDCContextFactory.java b/protocols/netty/src/main/java/org/apache/james/protocols/netty/ProtocolMDCContextFactory.java
index 7e1e9df..c308441 100644
--- a/protocols/netty/src/main/java/org/apache/james/protocols/netty/ProtocolMDCContextFactory.java
+++ b/protocols/netty/src/main/java/org/apache/james/protocols/netty/ProtocolMDCContextFactory.java
@@ -19,7 +19,6 @@
 
 package org.apache.james.protocols.netty;
 
-import java.io.Closeable;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Optional;
@@ -33,16 +32,22 @@ import org.jboss.netty.channel.ChannelHandlerContext;
 public interface ProtocolMDCContextFactory {
     class Standard implements ProtocolMDCContextFactory {
         @Override
-        public Closeable from(Protocol protocol, ChannelHandlerContext ctx) {
-            return mdcContext(protocol, ctx).build();
+        public MDCBuilder onBound(Protocol protocol, ChannelHandlerContext ctx) {
+            return mdcContext(protocol, ctx);
+        }
+
+        @Override
+        public MDCBuilder withContext(ProtocolSession protocolSession) {
+            return from(protocolSession);
         }
     }
 
-    Closeable from(Protocol protocol, ChannelHandlerContext ctx);
+    MDCBuilder onBound(Protocol protocol, ChannelHandlerContext ctx);
+
+    MDCBuilder withContext(ProtocolSession protocolSession);
 
     static MDCBuilder mdcContext(Protocol protocol, ChannelHandlerContext ctx) {
         return MDCBuilder.create()
-            .addToContext(from(ctx.getAttachment()))
             .addToContext(MDCBuilder.PROTOCOL, protocol.getName())
             .addToContext(MDCBuilder.IP, retrieveIp(ctx))
             .addToContext(MDCBuilder.HOST, retrieveHost(ctx));
@@ -66,7 +71,7 @@ public interface ProtocolMDCContextFactory {
         return remoteAddress.toString();
     }
 
-    private static MDCBuilder from(Object o) {
+    static MDCBuilder from(Object o) {
         return Optional.ofNullable(o)
             .filter(object -> object instanceof ProtocolSession)
             .map(object -> (ProtocolSession) object)
diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SMTPMDCContextFactory.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SMTPMDCContextFactory.java
index 97b1094..f07029c 100644
--- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SMTPMDCContextFactory.java
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SMTPMDCContextFactory.java
@@ -19,7 +19,6 @@
 
 package org.apache.james.protocols.smtp.core;
 
-import java.io.Closeable;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -33,17 +32,17 @@ import org.jboss.netty.channel.ChannelHandlerContext;
 
 public class SMTPMDCContextFactory implements ProtocolMDCContextFactory {
 
-    public Closeable from(Protocol protocol, ChannelHandlerContext ctx) {
-        return MDCBuilder.create()
-            .addToContext(ProtocolMDCContextFactory.mdcContext(protocol, ctx))
-            .addToContext(from(ctx.getAttachment()))
-            .build();
+    public MDCBuilder onBound(Protocol protocol, ChannelHandlerContext ctx) {
+        return ProtocolMDCContextFactory.mdcContext(protocol, ctx);
+    }
+
+    @Override
+    public MDCBuilder withContext(ProtocolSession protocolSession) {
+        return from(protocolSession);
     }
 
     public static MDCBuilder forSession(SMTPSession smtpSession) {
-        return MDCBuilder.create()
-            .addToContext(ProtocolMDCContextFactory.forSession(smtpSession))
-            .addToContext(forSMTPSession(smtpSession));
+        return forSMTPSession(smtpSession);
     }
 
     private MDCBuilder from(Object o) {
@@ -56,6 +55,7 @@ public class SMTPMDCContextFactory implements ProtocolMDCContextFactory {
 
     private static MDCBuilder forSMTPSession(SMTPSession smtpSession) {
         return MDCBuilder.create()
+            .addToContext(ProtocolMDCContextFactory.from(smtpSession))
             .addToContextIfPresent("ehlo", smtpSession.getAttachment(SMTPSession.CURRENT_HELO_NAME, ProtocolSession.State.Connection))
             .addToContextIfPresent("sender", smtpSession.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction)
                 .map(MaybeSender::asString))
diff --git a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
index 5255f07..f6a6dab 100644
--- a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
+++ b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
@@ -174,4 +174,9 @@ public class MDCBuilder {
             .forEach(MDC::remove);
     }
 
+    public MDCBuilder duplicate() {
+        return MDCBuilder.create()
+            .addToContext(this);
+    }
+
 }

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