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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
