From 05370a62dab0d4db0df0bde3ca202d281debaddd Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Wed, 27 Feb 2019 17:28:42 +0100 Subject: [PATCH 1/2] Fixed the bug where original host name is lost in ssl handshake. The driver resolves host address to IP addresses. In the scenarios with SNI, the original host name is required to be part of ssl handshake to indicate the server to connect to. Fixed the bug where driver uses IP address rather than the original host name in ssl handshake. --- .../driver/internal/BoltServerAddress.java | 26 ++++++++++++++----- .../async/NettyChannelInitializer.java | 2 +- .../driver/internal/cluster/DnsResolver.java | 2 +- .../neo4j/driver/v1/net/ServerAddress.java | 9 ++++++- .../async/NettyChannelInitializerTest.java | 4 +-- .../org/neo4j/driver/v1/util/Neo4jRunner.java | 2 +- 6 files changed, 33 insertions(+), 12 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java b/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java index 51a8c21387..0d478576a5 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java +++ b/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java @@ -23,6 +23,7 @@ import java.net.SocketAddress; import java.net.URI; import java.net.UnknownHostException; +import java.util.Objects; import org.neo4j.driver.v1.net.ServerAddress; @@ -36,7 +37,8 @@ public class BoltServerAddress implements ServerAddress public static final int DEFAULT_PORT = 7687; public static final BoltServerAddress LOCAL_DEFAULT = new BoltServerAddress( "localhost", DEFAULT_PORT ); - private final String host; + private final String originalHost; // This keeps the original host name provided by the user. + private final String host; // This could either be the same as originalHost or it is an IP address resolved from the original host. private final int port; private final String stringValue; @@ -52,6 +54,12 @@ public BoltServerAddress( URI uri ) public BoltServerAddress( String host, int port ) { + this( host, host, port ); + } + + public BoltServerAddress( String originalHost, String host, int port ) + { + this.originalHost = requireNonNull( originalHost, "original host" ); this.host = requireNonNull( host, "host" ); this.port = requireValidPort( port ); this.stringValue = String.format( "%s:%d", host, port ); @@ -76,13 +84,13 @@ public boolean equals( Object o ) return false; } BoltServerAddress that = (BoltServerAddress) o; - return port == that.port && host.equals( that.host ); + return port == that.port && originalHost.equals( that.originalHost ) && host.equals( that.host ); } @Override public int hashCode() { - return 31 * host.hashCode() + port; + return Objects.hash( originalHost, host, port ); } @Override @@ -112,14 +120,14 @@ public SocketAddress toSocketAddress() */ public BoltServerAddress resolve() throws UnknownHostException { - String hostAddress = InetAddress.getByName( host ).getHostAddress(); - if ( hostAddress.equals( host ) ) + String ipAddress = InetAddress.getByName( host ).getHostAddress(); + if ( ipAddress.equals( host ) ) { return this; } else { - return new BoltServerAddress( hostAddress, port ); + return new BoltServerAddress( host, ipAddress, port ); } } @@ -129,6 +137,12 @@ public String host() return host; } + @Override + public String originalHost() + { + return originalHost; + } + @Override public int port() { diff --git a/driver/src/main/java/org/neo4j/driver/internal/async/NettyChannelInitializer.java b/driver/src/main/java/org/neo4j/driver/internal/async/NettyChannelInitializer.java index c1edd12c65..12566f08ad 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/async/NettyChannelInitializer.java +++ b/driver/src/main/java/org/neo4j/driver/internal/async/NettyChannelInitializer.java @@ -77,7 +77,7 @@ private SslHandler createSslHandler() private SSLEngine createSslEngine() { SSLContext sslContext = securityPlan.sslContext(); - SSLEngine sslEngine = sslContext.createSSLEngine( address.host(), address.port() ); + SSLEngine sslEngine = sslContext.createSSLEngine( address.originalHost(), address.port() ); sslEngine.setUseClientMode( true ); if ( securityPlan.requiresHostnameVerification() ) { diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/DnsResolver.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/DnsResolver.java index 5af2a7df72..3dbec14c7d 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/DnsResolver.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/DnsResolver.java @@ -47,7 +47,7 @@ public Set resolve( ServerAddress initialRouter ) try { return Stream.of( InetAddress.getAllByName( initialRouter.host() ) ) - .map( address -> new BoltServerAddress( address.getHostAddress(), initialRouter.port() ) ) + .map( address -> new BoltServerAddress( initialRouter.host(), address.getHostAddress(), initialRouter.port() ) ) .collect( toSet() ); } catch ( UnknownHostException e ) diff --git a/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java b/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java index 1ee13a4620..4dd3f61dfb 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java +++ b/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java @@ -33,6 +33,13 @@ public interface ServerAddress */ String host(); + /** + * Returns the original host name of this {@link ServerAddress}. + * This value might different from {@link #host()} when the host is a resolved IP address. + * @return the original host name, never {@code null}. + */ + String originalHost(); + /** * Retrieve the port portion of this {@link ServerAddress}. * @@ -49,6 +56,6 @@ public interface ServerAddress */ static ServerAddress of( String host, int port ) { - return new BoltServerAddress( host, port ); + return new BoltServerAddress( host, host, port ); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/async/NettyChannelInitializerTest.java b/driver/src/test/java/org/neo4j/driver/internal/async/NettyChannelInitializerTest.java index a5ebdcb920..70f95073a0 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/async/NettyChannelInitializerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/async/NettyChannelInitializerTest.java @@ -114,7 +114,7 @@ void shouldUpdateChannelAttributes() @Test void shouldIncludeSniHostName() throws Exception { - BoltServerAddress address = new BoltServerAddress( "database.neo4j.com", 8989 ); + BoltServerAddress address = new BoltServerAddress( "database.neo4j.com", "10.0.0.18", 8989 ); NettyChannelInitializer initializer = new NettyChannelInitializer( address, trustAllCertificates(), 10000, Clock.SYSTEM, DEV_NULL_LOGGING ); initializer.initChannel( channel ); @@ -125,7 +125,7 @@ void shouldIncludeSniHostName() throws Exception List sniServerNames = sslParameters.getServerNames(); assertThat( sniServerNames, hasSize( 1 ) ); assertThat( sniServerNames.get( 0 ), instanceOf( SNIHostName.class ) ); - assertThat( ((SNIHostName) sniServerNames.get( 0 )).getAsciiName(), equalTo( address.host() ) ); + assertThat( ((SNIHostName) sniServerNames.get( 0 )).getAsciiName(), equalTo( address.originalHost() ) ); } @Test diff --git a/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java b/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java index eab267d845..bb481767ba 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java +++ b/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java @@ -86,7 +86,7 @@ public int boltPort() public BoltServerAddress boltAddress() { - return new BoltServerAddress( "localhost", boltPort() ); + return new BoltServerAddress( boltUri() ); } public URI boltUri() From e9e5a9396baf153804c6b2e18c4d669cf13d965b Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Thu, 28 Feb 2019 13:53:34 +0100 Subject: [PATCH 2/2] Fix some test failures due to changes to routing table procedure on read-replica in 4.0 Removed the changes to driver API --- .../java/org/neo4j/driver/internal/BoltServerAddress.java | 1 - .../java/org/neo4j/driver/internal/util/ServerVersion.java | 1 + .../main/java/org/neo4j/driver/v1/net/ServerAddress.java | 7 ------- .../java/org/neo4j/driver/internal/util/Neo4jFeature.java | 4 +++- .../neo4j/driver/v1/integration/CausalClusteringIT.java | 4 ++++ .../neo4j/driver/v1/integration/ConnectionHandlingIT.java | 2 +- 6 files changed, 9 insertions(+), 10 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java b/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java index 0d478576a5..a6a6316103 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java +++ b/driver/src/main/java/org/neo4j/driver/internal/BoltServerAddress.java @@ -137,7 +137,6 @@ public String host() return host; } - @Override public String originalHost() { return originalHost; diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/ServerVersion.java b/driver/src/main/java/org/neo4j/driver/internal/util/ServerVersion.java index d80ecd05e2..4fa75a40c8 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/ServerVersion.java +++ b/driver/src/main/java/org/neo4j/driver/internal/util/ServerVersion.java @@ -31,6 +31,7 @@ public class ServerVersion { public static final String NEO4J_PRODUCT = "Neo4j"; + public static final ServerVersion v4_0_0 = new ServerVersion( NEO4J_PRODUCT, 4, 0, 0 ); public static final ServerVersion v3_5_0 = new ServerVersion( NEO4J_PRODUCT, 3, 5, 0 ); public static final ServerVersion v3_4_0 = new ServerVersion( NEO4J_PRODUCT, 3, 4, 0 ); public static final ServerVersion v3_2_0 = new ServerVersion( NEO4J_PRODUCT, 3, 2, 0 ); diff --git a/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java b/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java index 4dd3f61dfb..880eef7a61 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java +++ b/driver/src/main/java/org/neo4j/driver/v1/net/ServerAddress.java @@ -33,13 +33,6 @@ public interface ServerAddress */ String host(); - /** - * Returns the original host name of this {@link ServerAddress}. - * This value might different from {@link #host()} when the host is a resolved IP address. - * @return the original host name, never {@code null}. - */ - String originalHost(); - /** * Retrieve the port portion of this {@link ServerAddress}. * diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/Neo4jFeature.java b/driver/src/test/java/org/neo4j/driver/internal/util/Neo4jFeature.java index 4c319f757a..a54522f1ba 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/Neo4jFeature.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/Neo4jFeature.java @@ -23,6 +23,7 @@ import static org.neo4j.driver.internal.util.ServerVersion.v3_2_0; import static org.neo4j.driver.internal.util.ServerVersion.v3_4_0; import static org.neo4j.driver.internal.util.ServerVersion.v3_5_0; +import static org.neo4j.driver.internal.util.ServerVersion.v4_0_0; public enum Neo4jFeature { @@ -36,7 +37,8 @@ public enum Neo4jFeature STATEMENT_RESULT_TIMINGS( v3_1_0 ), LIST_QUERIES_PROCEDURE( v3_1_0 ), CONNECTOR_LISTEN_ADDRESS_CONFIGURATION( v3_1_0 ), - BOLT_V3( v3_5_0 ); + BOLT_V3( v3_5_0 ), + BOLT_V4( v4_0_0 ); private final ServerVersion availableFromVersion; diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java index c569ce7887..7174c6151d 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java @@ -41,6 +41,7 @@ import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.retry.RetrySettings; import org.neo4j.driver.internal.util.ChannelTrackingDriverFactory; +import org.neo4j.driver.internal.util.DisabledOnNeo4jWith; import org.neo4j.driver.internal.util.FailingConnectionDriverFactory; import org.neo4j.driver.internal.util.FakeClock; import org.neo4j.driver.internal.util.ServerVersion; @@ -86,6 +87,7 @@ import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.internal.util.Matchers.connectionAcquisitionTimeoutError; import static org.neo4j.driver.internal.util.Neo4jFeature.BOLT_V3; +import static org.neo4j.driver.internal.util.Neo4jFeature.BOLT_V4; import static org.neo4j.driver.v1.Values.parameters; import static org.neo4j.driver.v1.util.DaemonThreadFactory.daemon; import static org.neo4j.driver.v1.util.TestUtil.await; @@ -137,6 +139,7 @@ void shouldExecuteReadAndWritesWhenDriverSuppliedWithAddressOfLeader() throws Ex } @Test + @DisabledOnNeo4jWith( BOLT_V4 ) void shouldExecuteReadAndWritesWhenRouterIsDiscovered() throws Exception { Cluster cluster = clusterRule.getCluster(); @@ -157,6 +160,7 @@ void shouldExecuteReadAndWritesWhenDriverSuppliedWithAddressOfFollower() throws } @Test + @DisabledOnNeo4jWith( BOLT_V4 ) void sessionCreationShouldFailIfCallingDiscoveryProcedureOnEdgeServer() { Cluster cluster = clusterRule.getCluster(); diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java index 137a99f946..bf771c9c68 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java @@ -189,7 +189,7 @@ void connectionUsedForSessionRunReturnedToThePoolWhenServerErrorDuringResultFetc Connection connection1 = connectionPool.lastAcquiredConnectionSpy; verify( connection1, never() ).release(); - assertThrows( ClientException.class, result::hasNext ); + assertThrows( ClientException.class, result::consume ); Connection connection2 = connectionPool.lastAcquiredConnectionSpy; assertSame( connection1, connection2 );