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 d43cfea12cd3e8db556edf84c06fc7e555ad940d
Author: Benoit TELLIER <btell...@linagora.com>
AuthorDate: Wed Nov 6 09:46:10 2024 +0100
[ENHANCEMENT] Ensure the MDC context is carried over upon IMAP parsing
errors
This would ease linking and identifying IMAP parsing issues
---
.../apache/james/imap/api/process/ImapSession.java | 20 ++++++++++++++++++++
.../decode/base/AbstractImapCommandParser.java | 2 +-
.../james/imap/decode/main/DefaultImapDecoder.java | 4 ++--
.../imapserver/netty/HAProxyMessageHandler.java | 2 +-
.../james/imapserver/netty/IMAPMDCContext.java | 11 -----------
.../netty/ImapChannelUpstreamHandler.java | 11 +++--------
.../james/imapserver/netty/NettyImapSession.java | 22 ++++++++++++++++++++++
7 files changed, 49 insertions(+), 23 deletions(-)
diff --git
a/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
b/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
index fa6b508551..a95ef8a04a 100644
---
a/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
+++
b/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
@@ -19,6 +19,8 @@
package org.apache.james.imap.api.process;
+import java.io.Closeable;
+import java.io.IOException;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.util.Objects;
@@ -32,6 +34,7 @@ import org.apache.james.imap.api.ImapSessionState;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.protocols.api.CommandDetectionSession;
import org.apache.james.protocols.api.OidcSASLConfiguration;
+import org.apache.james.util.MDCBuilder;
import reactor.core.publisher.Mono;
@@ -43,6 +46,8 @@ import reactor.core.publisher.Mono;
* @version $Revision: 109034 $
*/
public interface ImapSession extends CommandDetectionSession {
+ String MDC_KEY = "bound_MDC";
+
class SessionId {
private static final RandomStringGenerator RANDOM_STRING_GENERATOR =
new RandomStringGenerator.Builder()
.withinRange('a', 'z')
@@ -251,6 +256,21 @@ public interface ImapSession extends
CommandDetectionSession {
boolean supportsOAuth();
+ default void withMDC(Runnable runnable) {
+ try (Closeable c = mdc().build()) {
+ runnable.run();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ default MDCBuilder mdc() {
+ if (getAttribute(MDC_KEY) instanceof MDCBuilder mdcBuilder) {
+ return mdcBuilder;
+ }
+ return MDCBuilder.create();
+ }
+
/**
* Return the {@link InetSocketAddress} of the remote peer
*/
diff --git
a/protocols/imap/src/main/java/org/apache/james/imap/decode/base/AbstractImapCommandParser.java
b/protocols/imap/src/main/java/org/apache/james/imap/decode/base/AbstractImapCommandParser.java
index 3e74686dd2..30f1336116 100644
---
a/protocols/imap/src/main/java/org/apache/james/imap/decode/base/AbstractImapCommandParser.java
+++
b/protocols/imap/src/main/java/org/apache/james/imap/decode/base/AbstractImapCommandParser.java
@@ -71,7 +71,7 @@ public abstract class AbstractImapCommandParser implements
ImapCommandParser {
try {
return decode(request, tag, session);
} catch (DecodingException e) {
- LOGGER.info("Cannot parse protocol ", e);
+ session.withMDC(() -> LOGGER.info("Cannot parse protocol ",
e));
return statusResponseFactory.taggedBad(tag, command,
e.getKey());
}
}
diff --git
a/protocols/imap/src/main/java/org/apache/james/imap/decode/main/DefaultImapDecoder.java
b/protocols/imap/src/main/java/org/apache/james/imap/decode/main/DefaultImapDecoder.java
index 7f3efdba0b..7de67dccef 100644
---
a/protocols/imap/src/main/java/org/apache/james/imap/decode/main/DefaultImapDecoder.java
+++
b/protocols/imap/src/main/java/org/apache/james/imap/decode/main/DefaultImapDecoder.java
@@ -70,7 +70,7 @@ public class DefaultImapDecoder implements ImapDecoder {
Tag tag = request.tag();
return decodeCommandTagged(request, tag, session);
} catch (DecodingException e) {
- LOGGER.debug("Cannot parse tag", e);
+ session.withMDC(() -> LOGGER.debug("Cannot parse tag", e));
return unknownCommand(null, session);
}
}
@@ -80,7 +80,7 @@ public class DefaultImapDecoder implements ImapDecoder {
String commandName = request.atom();
return decodeCommandNamed(request, tag, commandName, session);
} catch (DecodingException e) {
- LOGGER.info("Error during initial request parsing", e);
+ session.withMDC(() -> LOGGER.info("Error during initial request
parsing", e));
return unknownCommand(tag, session);
}
}
diff --git
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java
index 5a89b28688..463fbadf4b 100644
---
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java
+++
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java
@@ -19,7 +19,7 @@
package org.apache.james.imapserver.netty;
-import static
org.apache.james.imapserver.netty.ImapChannelUpstreamHandler.MDC_KEY;
+import static org.apache.james.imap.api.process.ImapSession.MDC_KEY;
import java.net.InetSocketAddress;
import java.util.Set;
diff --git
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java
index 1631f00bde..a0ec4d3b88 100644
---
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java
+++
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java
@@ -23,8 +23,6 @@ import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Optional;
-import org.apache.james.core.Username;
-import org.apache.james.imap.api.process.ImapSession;
import org.apache.james.protocols.netty.ProtocolMDCContextFactory;
import org.apache.james.util.MDCBuilder;
@@ -71,13 +69,4 @@ public class IMAPMDCContext {
}
return remoteAddress.toString();
}
-
- public static MDCBuilder from(ImapSession imapSession) {
- return MDCBuilder.create()
- .addToContext(MDCBuilder.USER,
Optional.ofNullable(imapSession.getUserName())
- .map(Username::asString)
- .orElse(""))
- .addToContextIfPresent("selectedMailbox",
Optional.ofNullable(imapSession.getSelected())
- .map(selectedMailbox ->
selectedMailbox.getMailboxId().serialize()));
- }
}
diff --git
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java
index 72a879205c..938938cee2 100644
---
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java
+++
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java
@@ -18,6 +18,7 @@
****************************************************************/
package org.apache.james.imapserver.netty;
+import static org.apache.james.imap.api.process.ImapSession.MDC_KEY;
import static
org.apache.james.imapserver.netty.IMAPServer.AuthenticationConfiguration;
import java.io.Closeable;
@@ -78,7 +79,6 @@ import reactor.core.publisher.Mono;
@ChannelHandler.Sharable
public class ImapChannelUpstreamHandler extends ChannelInboundHandlerAdapter
implements NettyConstants {
private static final Logger LOGGER =
LoggerFactory.getLogger(ImapChannelUpstreamHandler.class);
- public static final String MDC_KEY = "bound_MDC";
public static class ImapChannelUpstreamHandlerBuilder {
private String hello;
@@ -254,12 +254,7 @@ public class ImapChannelUpstreamHandler extends
ChannelInboundHandlerAdapter imp
private MDCBuilder mdc(ImapSession imapSession) {
return Optional.ofNullable(imapSession)
- .map(session -> {
- MDCBuilder boundMDC = (MDCBuilder)
session.getAttribute(MDC_KEY);
-
- return IMAPMDCContext.from(session)
- .addToContext(boundMDC);
- })
+ .map(ImapSession::mdc)
.orElseGet(MDCBuilder::create);
}
@@ -297,7 +292,7 @@ public class ImapChannelUpstreamHandler extends
ChannelInboundHandlerAdapter imp
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause)
throws Exception {
ImapSession imapSession =
ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).getAndSet(null);
String username = retrieveUsername(imapSession);
- try (Closeable closeable = mdc(ctx).build()) {
+ try (Closeable closeable = mdc(imapSession).build()) {
if (cause instanceof SocketException) {
LOGGER.info("Socket exception encountered for user {}: {}",
username, cause.getMessage());
} else if (isSslHandshkeException(cause)) {
diff --git
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
index ff61f4e692..22d6614c89 100644
---
a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
+++
b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
@@ -30,6 +30,7 @@ import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLSession;
import org.apache.commons.lang3.NotImplementedException;
+import org.apache.james.core.Username;
import org.apache.james.imap.api.ImapSessionState;
import org.apache.james.imap.api.process.ImapLineHandler;
import org.apache.james.imap.api.process.ImapSession;
@@ -40,6 +41,7 @@ import org.apache.james.mailbox.MailboxSession;
import org.apache.james.protocols.api.OidcSASLConfiguration;
import org.apache.james.protocols.netty.Encryption;
import org.apache.james.protocols.netty.LineHandlerAware;
+import org.apache.james.util.MDCBuilder;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
@@ -142,6 +144,7 @@ public class NettyImapSession implements ImapSession,
NettyConstants {
.orElse(Mono.empty()));
}
+
@Override
public MailboxSession getMailboxSession() {
return mailboxSession;
@@ -150,6 +153,25 @@ public class NettyImapSession implements ImapSession,
NettyConstants {
@Override
public void setMailboxSession(MailboxSession mailboxSession) {
this.mailboxSession = mailboxSession;
+ addUserToMDC(mailboxSession);
+ }
+
+ private void addUserToMDC(MailboxSession mailboxSession) {
+ setAttribute(MDC_KEY, mdc()
+ .addToContext(MDCBuilder.USER,
Optional.ofNullable(mailboxSession.getUser())
+ .map(Username::asString)
+ .orElse("")));
+ }
+
+ @Override
+ public MDCBuilder mdc() {
+ SelectedMailbox mailbox = selectedMailbox.get();
+ if (mailbox != null) {
+ return MDCBuilder.create()
+ .addToContext(ImapSession.super.mdc())
+ .addToContext("selectedMailbox",
mailbox.getMailboxId().serialize());
+ }
+ return ImapSession.super.mdc();
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
