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 2022/03/22 02:01:41 UTC

[james-project] 10/29: JAMES-3715 IMAP: Avoid a deadlock upon COMPRESS

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 acb5a9282eee20ebaadc803d3d259427502e49b6
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Mar 7 16:10:27 2022 +0700

    JAMES-3715 IMAP: Avoid a deadlock upon COMPRESS
    
    The initial COMPRESS OK response was never sent because it was sent out of
    the event loop.
    
    We explicitly write it from the event loop so that the client is guarantied to receive it.
    
    We wrap the answer inside the "disabled-auto-read" to guaranty that a
    "too quick client" do not end up with a race condition...
---
 .../apache/james/imap/api/process/ImapSession.java  |  2 +-
 .../apache/james/imap/encode/FakeImapSession.java   |  2 +-
 .../james/imap/processor/CompressProcessor.java     |  6 ++++--
 .../james/imapserver/netty/NettyImapSession.java    | 21 ++++++++++++++-------
 4 files changed, 20 insertions(+), 11 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 4444ed4..1f793c2 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
@@ -193,7 +193,7 @@ public interface ImapSession extends CommandDetectionSession {
      * 
      * @return success
      */
-    boolean startCompression();
+    boolean startCompression(ImmutableStatusResponse response);
 
     /**
      * Push in a new {@link ImapLineHandler} which is called for the next line received
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java b/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
index cb1de55..4f2b5ae 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
@@ -134,7 +134,7 @@ public class FakeImapSession implements ImapSession {
     }
 
     @Override
-    public boolean startCompression() {
+    public boolean startCompression(ImmutableStatusResponse response) {
         return false;
     }
 
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/CompressProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/CompressProcessor.java
index 449a545..566d1a0 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/CompressProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/CompressProcessor.java
@@ -25,10 +25,12 @@ import java.util.List;
 import org.apache.james.imap.api.ImapConstants;
 import org.apache.james.imap.api.display.HumanReadableText;
 import org.apache.james.imap.api.message.Capability;
+import org.apache.james.imap.api.message.response.StatusResponse;
 import org.apache.james.imap.api.message.response.StatusResponseFactory;
 import org.apache.james.imap.api.process.ImapProcessor;
 import org.apache.james.imap.api.process.ImapSession;
 import org.apache.james.imap.message.request.CompressRequest;
+import org.apache.james.imap.message.response.ImmutableStatusResponse;
 import org.apache.james.imap.processor.base.AbstractChainedProcessor;
 import org.apache.james.util.MDCBuilder;
 
@@ -55,9 +57,9 @@ public class CompressProcessor extends AbstractChainedProcessor<CompressRequest>
                 if (request.getAlgorithm().equalsIgnoreCase(ALGO) == false) {
                     responder.respond(factory.taggedBad(request.getTag(), request.getCommand(), HumanReadableText.ILLEGAL_ARGUMENTS));
                 } else {
-                    responder.respond(factory.taggedOk(request.getTag(), request.getCommand(), HumanReadableText.DEFLATE_ACTIVE));
+                    StatusResponse response = factory.taggedOk(request.getTag(), request.getCommand(), HumanReadableText.DEFLATE_ACTIVE);
 
-                    if (session.startCompression()) {
+                    if (session.startCompression((ImmutableStatusResponse) response)) {
                         session.setAttribute(COMPRESSED, true);
                     }
                 }
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 2e6c109..d92ff14 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
@@ -48,6 +48,8 @@ import io.netty.handler.codec.compression.ZlibWrapper;
 import io.netty.handler.ssl.SslHandler;
 
 public class NettyImapSession implements ImapSession, NettyConstants {
+    private static final int BUFFER_SIZE = 2048;
+
     private ImapSessionState state = ImapSessionState.NON_AUTHENTICATED;
     private SelectedMailbox selectedMailbox;
     private final Map<String, Object> attributesByKey = new HashMap<>();
@@ -155,12 +157,7 @@ public class NettyImapSession implements ImapSession, NettyConstants {
             return false;
         }
         channel.config().setAutoRead(false);
-        try {
-            new StatusResponseEncoder(new DefaultLocalizer()).encode(statusResponse,
-                new ImapResponseComposerImpl(new EventLoopImapResponseWriter(channel), 2048));
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
+        writeOnTheEventLoop(statusResponse);
 
         SslHandler filter = new SslHandler(secure.createSSLEngine(), false);
 
@@ -172,6 +169,15 @@ public class NettyImapSession implements ImapSession, NettyConstants {
         return true;
     }
 
+    private void writeOnTheEventLoop(ImmutableStatusResponse statusResponse) {
+        try {
+            new StatusResponseEncoder(new DefaultLocalizer()).encode(statusResponse,
+                new ImapResponseComposerImpl(new EventLoopImapResponseWriter(channel), BUFFER_SIZE));
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     public static class EventLoopImapResponseWriter implements ImapResponseWriter {
         private final Channel channel;
 
@@ -204,12 +210,13 @@ public class NettyImapSession implements ImapSession, NettyConstants {
     }
 
     @Override
-    public boolean startCompression() {
+    public boolean startCompression(ImmutableStatusResponse response) {
         if (!isCompressionSupported()) {
             return false;
         }
 
         channel.config().setAutoRead(false);
+        writeOnTheEventLoop(response);
         ZlibDecoder decoder = new JZlibDecoder(ZlibWrapper.NONE);
         ZlibEncoder encoder = new JZlibEncoder(ZlibWrapper.NONE, 5);
 

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