You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/03/16 11:23:23 UTC

[GitHub] [james-project] chibenwa opened a new pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

chibenwa opened a new pull request #920:
URL: https://github.com/apache/james-project/pull/920


   ## Why
   
   ![Screenshot from 2022-03-16 16-12-43](https://user-images.githubusercontent.com/6928740/158579162-2484edda-9516-4245-8f13-fa7a639ac496.png)
   
   This reliable reproduces the issue in the Netty4 pull request.


-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] vttranlina commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r829650011



##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       should we have a property for all netty dependency?

##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       should we have a version property for all netty dependency?




-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] quantranhong1999 commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r828918367



##########
File path: server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
##########
@@ -1026,6 +1041,241 @@ void searchingASingleUTF8CriterionShouldComplete() throws Exception {
         }
     }
 
+    public static class BlindTrustManager implements X509TrustManager {
+        public X509Certificate[] getAcceptedIssuers() {
+            return null;
+        }
+
+        public void checkClientTrusted(X509Certificate[] chain, String authType) {
+
+        }
+
+        public void checkServerTrusted(X509Certificate[] chain, String authType) {
+
+
+        }
+    }
+
+    @Nested
+    class IdleSSL {
+        IMAPServer imapServer;
+        private MailboxSession mailboxSession;
+        private MessageManager inbox;
+        private Socket clientConnection;
+
+        @BeforeEach
+        void beforeEach() throws Exception {
+            imapServer = createImapServer("imapServerSSL.xml");
+            int port = imapServer.getListenAddresses().get(0).getPort();
+            mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+            memoryIntegrationResources.getMailboxManager()
+                .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+            inbox = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+            SSLContext ctx = SSLContext.getInstance("TLS");
+            ctx.init(null, new TrustManager[] { new BlindTrustManager() }, null);
+            clientConnection =ctx.getSocketFactory().createSocket();
+            clientConnection.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            byte[] buffer = new byte[8193];
+            clientConnection.getInputStream().read(buffer);
+        }
+
+        @AfterEach
+        void tearDown() throws Exception {
+            clientConnection.close();
+            imapServer.destroy();
+        }
+
+        @Test
+        void idleShouldSendInitialContinuation() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\n".getBytes(StandardCharsets.UTF_8));
+
+
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("+ Idling")))
+                    .isNotNull());
+        }
+
+        @Test
+        void idleShouldBeInterruptible() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write(("a3 IDLE\r\n".getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(clientConnection, s -> s.contains("+ Idling"));
+
+            clientConnection.getOutputStream().write("DONE\r\n".getBytes(StandardCharsets.UTF_8));
+
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("a3 OK IDLE completed.")))
+                    .isNotNull());
+        }
+
+        @Test
+        void idleShouldBeInterruptibleWhenBatched() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\nDONE\r\n".getBytes(StandardCharsets.UTF_8));
+
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("a3 OK IDLE completed.")))
+                    .isNotNull());
+        }
+
+        @Test
+        void idleResponsesShouldBeOrdered() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\nDONE\r\n".getBytes(StandardCharsets.UTF_8));
+
+            // Assert continuation is sent before IDLE completion result
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("a3 OK IDLE completed.")))
+                    .filteredOn(s -> s.contains("+ Idling"))
+                    .hasSize(1));
+        }
+
+        @Test
+        void idleShouldReturnUnderstandableErrorMessageWhenBadDone() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\nBAD\r\n".getBytes(StandardCharsets.UTF_8));
+
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("a3 BAD IDLE failed. Continuation for IMAP IDLE was not understood. Expected 'DONE', got 'BAD'.")))
+                    .isNotNull());
+        }
+
+        // Repeated run to detect more reliably data races
+        @RepeatedTest(50)
+        void idleShouldReturnUpdates() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("+ Idling"));
+
+            inbox.appendMessage(MessageManager.AppendCommand.builder().build("h: value\r\n\r\nbody".getBytes()), mailboxSession);
+
+            Awaitility.await().atMost(Duration.ofSeconds(2)).untilAsserted(() ->
+                assertThat(readStringUntil(clientConnection, s -> s.contains("* 1 EXISTS")))
+                    .isNotNull());
+        }
+
+        private byte[] readBytes(Socket channel) throws IOException {
+            byte[] buffer = new byte[8193];
+            final int read = channel.getInputStream().read(buffer);
+            final String s = new String(buffer, 0, read, StandardCharsets.US_ASCII);
+            System.out.println("C: " + s);

Review comment:
       Please remove this after finishing debug

##########
File path: server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
##########
@@ -1026,6 +1041,241 @@ void searchingASingleUTF8CriterionShouldComplete() throws Exception {
         }
     }
 
+    public static class BlindTrustManager implements X509TrustManager {
+        public X509Certificate[] getAcceptedIssuers() {
+            return null;
+        }
+
+        public void checkClientTrusted(X509Certificate[] chain, String authType) {
+
+        }
+
+        public void checkServerTrusted(X509Certificate[] chain, String authType) {
+
+
+        }
+    }
+
+    @Nested
+    class IdleSSL {
+        IMAPServer imapServer;
+        private MailboxSession mailboxSession;
+        private MessageManager inbox;
+        private Socket clientConnection;
+
+        @BeforeEach
+        void beforeEach() throws Exception {
+            imapServer = createImapServer("imapServerSSL.xml");
+            int port = imapServer.getListenAddresses().get(0).getPort();
+            mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+            memoryIntegrationResources.getMailboxManager()
+                .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+            inbox = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+            SSLContext ctx = SSLContext.getInstance("TLS");
+            ctx.init(null, new TrustManager[] { new BlindTrustManager() }, null);
+            clientConnection =ctx.getSocketFactory().createSocket();
+            clientConnection.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            byte[] buffer = new byte[8193];
+            clientConnection.getInputStream().read(buffer);
+        }
+
+        @AfterEach
+        void tearDown() throws Exception {
+            clientConnection.close();
+            imapServer.destroy();
+        }
+
+        @Test
+        void idleShouldSendInitialContinuation() throws Exception {
+            clientConnection.getOutputStream().write(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8));
+            readBytes(clientConnection);
+
+            clientConnection.getOutputStream().write("a2 SELECT INBOX\r\n".getBytes(StandardCharsets.UTF_8));
+            readStringUntil(clientConnection, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            clientConnection.getOutputStream().write("a3 IDLE\r\n".getBytes(StandardCharsets.UTF_8));
+
+

Review comment:
       We could reduce a line here?

##########
File path: server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
##########
@@ -1026,6 +1041,241 @@ void searchingASingleUTF8CriterionShouldComplete() throws Exception {
         }
     }
 
+    public static class BlindTrustManager implements X509TrustManager {
+        public X509Certificate[] getAcceptedIssuers() {
+            return null;
+        }
+
+        public void checkClientTrusted(X509Certificate[] chain, String authType) {
+
+        }
+
+        public void checkServerTrusted(X509Certificate[] chain, String authType) {
+
+
+        }
+    }
+
+    @Nested
+    class IdleSSL {
+        IMAPServer imapServer;
+        private MailboxSession mailboxSession;
+        private MessageManager inbox;
+        private Socket clientConnection;
+
+        @BeforeEach
+        void beforeEach() throws Exception {
+            imapServer = createImapServer("imapServerSSL.xml");
+            int port = imapServer.getListenAddresses().get(0).getPort();
+            mailboxSession = memoryIntegrationResources.getMailboxManager().createSystemSession(USER);
+            memoryIntegrationResources.getMailboxManager()
+                .createMailbox(MailboxPath.inbox(USER), mailboxSession);
+            inbox = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession);
+
+            SSLContext ctx = SSLContext.getInstance("TLS");
+            ctx.init(null, new TrustManager[] { new BlindTrustManager() }, null);
+            clientConnection =ctx.getSocketFactory().createSocket();

Review comment:
       a little space here ^^




-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] chibenwa commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r829661371



##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       This is a testing dependency and this will disapear with the netty4 migration...




-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] chibenwa merged pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #920:
URL: https://github.com/apache/james-project/pull/920


   


-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] chibenwa commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r829661371



##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       This is a testing dependency and this will disapear with the netty4 migration...




-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] Arsnael commented on pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #920:
URL: https://github.com/apache/james-project/pull/920#issuecomment-1070669185


   ```
   org.apache.james.imapserver.netty.IMAPServerTest$AuthenticationRequireSSL.loginShouldFailWhenRequireSSLAndUnEncryptedChannel
   
   java.lang.RuntimeException: Unable to register mbean
   	at org.apache.james.imapserver.netty.IMAPServerTest$AuthenticationRequireSSL.loginShouldFailWhenRequireSSLAndUnEncryptedChannel(IMAPServerTest.java:775)
   Caused by: javax.management.InstanceAlreadyExistsException: org.apache.james:type=server,name=imapserver,sub-type=threadpool,threadpool=imapserver-executor
   	at org.apache.james.imapserver.netty.IMAPServerTest$AuthenticationRequireSSL.loginShouldFailWhenRequireSSLAndUnEncryptedChannel(IMAPServerTest.java:775)
   ```


-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] vttranlina commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r829650011



##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       should we have a property for all netty dependency?




-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] chibenwa merged pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #920:
URL: https://github.com/apache/james-project/pull/920


   


-- 
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-unsubscribe@james.apache.org

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



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


[GitHub] [james-project] vttranlina commented on a change in pull request #920: JAMES-3715 Add IMAP IDLE tests when combined with SSL, COMPRESS

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #920:
URL: https://github.com/apache/james-project/pull/920#discussion_r829650011



##########
File path: server/protocols/protocols-imap4/pom.xml
##########
@@ -131,6 +131,16 @@
             <groupId>io.netty</groupId>
             <artifactId>netty</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty-all</artifactId>
+            <version>4.1.72.Final</version>

Review comment:
       should we have a version property for all netty dependency?




-- 
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-unsubscribe@james.apache.org

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



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