zookeeper-commits mailing list archives

This is an automated email from the ASF dual-hosted git repository.

andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 6b6ff1d  ZOOKEEPER-3455: fix UnifiedServerSocketTest on jdk 13
6b6ff1d is described below

commit 6b6ff1d11009d1301eb48d7801175d6af7dbf3fe
Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com>
AuthorDate: Fri Aug 2 13:55:01 2019 +0200

    ZOOKEEPER-3455: fix UnifiedServerSocketTest on jdk 13
    
    The `UnifiedServerSocketTest.testConnectWithoutSSLToStrictServer` fails on OpenJDK 13
because in the new default socket implementation (https://openjdk.java.net/jeps/353) the `NioSocketImpl.getInputStream.read()`
behaves differently than the old `SocketInputStream.read()`.
    
    A workaround could be to execute the tests using the `-Djdk.net.usePlainSocketImpl` system
property (hardcoding it in the maven / ant configs), which enforces the usage of the old socket
implementation in JDK 13. But I preferred instead to make the test compatible with both older
and newer JDK, so it should succeed even if someone is executing it outside of our build environment.
    
    Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com>
    
    Reviewers: andor@apache.org
    
    Closes #1029 from symat/master
---
 .../server/quorum/UnifiedServerSocketTest.java         | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
index ddc05dc..b74fcda 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
@@ -212,6 +212,10 @@ public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
         synchronized byte[] getDataFromClient(int index) {
             return dataFromClients.get(index);
         }
+
+        synchronized boolean receivedAnyDataFromClient() {
+            return !dataFromClients.isEmpty();
+        }
     }
 
     private SSLSocket connectWithSSL() throws IOException, X509Exception, InterruptedException
{
@@ -405,13 +409,23 @@ public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
         socket.getOutputStream().flush();
         byte[] buf = new byte[DATA_TO_CLIENT.length];
         try {
-            socket.getInputStream().read(buf, 0, buf.length);
+            int bytesRead = socket.getInputStream().read(buf, 0, buf.length);
+            if(bytesRead == -1) {
+                // Using the NioSocketImpl after JDK 13, the expected behaviour on the client
side
+                // is to reach the end of the stream (bytesRead == -1), without a socket
exception.
+                return;
+            }
         } catch (SocketException e) {
-            // We expect the other end to hang up the connection
+            // Using the old PlainSocketImpl (prior to JDK 13) we expect to get Socket Exception
             return;
         } finally {
             forceClose(socket);
             serverThread.shutdown(TIMEOUT);
+
+            // independently of the client socket implementation details, we always make
sure the
+            // server didn't receive any data during the test
+            Assert.assertFalse("The strict server accepted connection without SSL.",
+                               serverThread.receivedAnyDataFromClient());
         }
         Assert.fail("Expected server to hang up the connection. Read from server succeeded
unexpectedly.");
     }