Since Server v4.0 the server agent received in response to the Hello message is unreliable. Instead, since the server and Bolt protocol versions are aligned, we use the protocol version instead for all connections to Server 4.0 and higher (#708)

Since Server v4.0 the server agent received in response to the Hello message is unreliable. Instead, since the server and Bolt protocol versions are aligned, we use the protocol version instead for all connections to Server 4.0 and higher.

Note: A future refactoring is required when support for 3.5 is dropped. The name serverVersion filed of the netty channel no longer reflects its true purpose since it essentially becomes protocolVersion. We keep the current naming for now since the split in meaning between 3.5 and 4.X+

Author: gjmwoods

driver/src/main/java/org/neo4j/driver/internal/handlers/HelloResponseHandler.java

@@ -37,20 +37,30 @@ public class HelloResponseHandler implements ResponseHandler
 
     private final ChannelPromise connectionInitializedPromise;
     private final Channel channel;
+    private final int protocolVersion;
 
-    public HelloResponseHandler( ChannelPromise connectionInitializedPromise )
+    public HelloResponseHandler( ChannelPromise connectionInitializedPromise, int protocolVersion )
     {
         this.connectionInitializedPromise = connectionInitializedPromise;
         this.channel = connectionInitializedPromise.channel();
+        this.protocolVersion = protocolVersion;
     }
 
     @Override
     public void onSuccess( Map<String,Value> metadata )
     {
         try
         {
-            ServerVersion serverVersion = extractNeo4jServerVersion( metadata );
-            setServerVersion( channel, serverVersion );
+            // From Server V4 extracting server from metadata in the success message is unreliable
+            // so we fix the Server version against the Bolt Protocol version for Server V4 and above.
+            if ( protocolVersion == 4 )
+            {
+                setServerVersion( channel, ServerVersion.v4_0_0 );
+            }
+            else
+            {
+                setServerVersion( channel, extractNeo4jServerVersion( metadata ) );
+            }
 
             String connectionId = extractConnectionId( metadata );
             setConnectionId( channel, connectionId );

driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java

@@ -80,7 +80,7 @@ public void initializeChannel( String userAgent, Map<String,Value> authToken, Ch
         Channel channel = channelInitializedPromise.channel();
 
         HelloMessage message = new HelloMessage( userAgent, authToken );
-        HelloResponseHandler handler = new HelloResponseHandler( channelInitializedPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelInitializedPromise, version() );
 
         messageDispatcher( channel ).enqueue( handler );
         channel.writeAndFlush( message, channel.voidPromise() );

driver/src/test/java/org/neo4j/driver/internal/handlers/HelloResponseHandlerTest.java

@@ -37,6 +37,7 @@
 import org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher;
 import org.neo4j.driver.internal.async.outbound.OutboundMessageHandler;
 import org.neo4j.driver.internal.messaging.v1.MessageFormatV1;
+import org.neo4j.driver.internal.util.ServerVersion;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -73,7 +74,7 @@ void tearDown()
     void shouldSetServerVersionOnChannel()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( anyServerVersion(), "bolt-1" );
         handler.onSuccess( metadata );
@@ -86,7 +87,7 @@ void shouldSetServerVersionOnChannel()
     void shouldThrowWhenServerVersionNotReturned()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( null, "bolt-1" );
         assertThrows( UntrustedServerException.class, () -> handler.onSuccess( metadata ) );
@@ -99,7 +100,7 @@ void shouldThrowWhenServerVersionNotReturned()
     void shouldThrowWhenServerVersionIsNull()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( Values.NULL, "bolt-x" );
         assertThrows( UntrustedServerException.class, () -> handler.onSuccess( metadata ) );
@@ -112,7 +113,7 @@ void shouldThrowWhenServerVersionIsNull()
     void shouldThrowWhenServerVersionCantBeParsed()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( "WrongServerVersion", "bolt-x" );
         assertThrows( IllegalArgumentException.class, () -> handler.onSuccess( metadata ) );
@@ -121,11 +122,25 @@ void shouldThrowWhenServerVersionCantBeParsed()
         assertTrue( channel.closeFuture().isDone() ); // channel was closed
     }
 
+    @Test
+    void shouldUseProtocolVersionForServerVersionWhenConnectedWithBoltV4()
+    {
+        ChannelPromise channelPromise = channel.newPromise();
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 4 );
+
+        // server used in metadata should be ignored
+        Map<String,Value> metadata = metadata( ServerVersion.vInDev, "bolt-1" );
+        handler.onSuccess( metadata );
+
+        assertTrue( channelPromise.isSuccess() );
+        assertEquals( ServerVersion.v4_0_0, serverVersion( channel ) );
+    }
+
     @Test
     void shouldSetConnectionIdOnChannel()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( anyServerVersion(), "bolt-42" );
         handler.onSuccess( metadata );
@@ -138,7 +153,7 @@ void shouldSetConnectionIdOnChannel()
     void shouldThrowWhenConnectionIdNotReturned()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( anyServerVersion(), null );
         assertThrows( IllegalStateException.class, () -> handler.onSuccess( metadata ) );
@@ -151,7 +166,7 @@ void shouldThrowWhenConnectionIdNotReturned()
     void shouldThrowWhenConnectionIdIsNull()
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         Map<String,Value> metadata = metadata( anyServerVersion(), Values.NULL );
         assertThrows( IllegalStateException.class, () -> handler.onSuccess( metadata ) );
@@ -164,7 +179,7 @@ void shouldThrowWhenConnectionIdIsNull()
     void shouldCloseChannelOnFailure() throws Exception
     {
         ChannelPromise channelPromise = channel.newPromise();
-        HelloResponseHandler handler = new HelloResponseHandler( channelPromise );
+        HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 );
 
         RuntimeException error = new RuntimeException( "Hi!" );
         handler.onFailure( error );