diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java index 0f2fc29c84..a4babae078 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java @@ -78,9 +78,7 @@ public synchronized Set update( ClusterComposition cluster ) @Override public synchronized void forget( BoltServerAddress address ) { - // Don't remove it from the set of routers, since that might mean we lose our ability to re-discover, - // just remove it from the set of readers and writers, so that we don't use it for actual work without - // performing discovery first. + routers.remove( address ); readers.remove( address ); writers.remove( address ); } @@ -115,11 +113,6 @@ public void removeWriter( BoltServerAddress toRemove ) writers.remove( toRemove ); } - @Override - public void removeRouter( BoltServerAddress toRemove ) - { - routers.remove( toRemove ); - } @Override public String toString() 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 new file mode 100644 index 0000000000..8864b0402c --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/DnsResolver.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal.cluster; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.HashSet; +import java.util.Set; + +import org.neo4j.driver.internal.net.BoltServerAddress; +import org.neo4j.driver.v1.Logger; + +public class DnsResolver implements HostNameResolver +{ + private final Logger logger; + + public DnsResolver( Logger logger ) + { + this.logger = logger; + } + + @Override + public Set resolve( BoltServerAddress initialRouter ) + { + Set addresses = new HashSet<>(); + try + { + InetAddress[] ipAddresses = InetAddress.getAllByName( initialRouter.host() ); + + for ( InetAddress ipAddress : ipAddresses ) + { + addresses.add( new BoltServerAddress( ipAddress.getHostAddress(), initialRouter.port() ) ); + } + + return addresses; + } + catch ( UnknownHostException e ) + { + logger.error( "Failed to resolve URI `" + initialRouter + "` to IPs due to error: " + e.getMessage(), e ); + + addresses.add( initialRouter ); + return addresses; + } + } +} diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/HostNameResolver.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/HostNameResolver.java new file mode 100644 index 0000000000..47591e7368 --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/HostNameResolver.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal.cluster; + +import java.util.Set; + +import org.neo4j.driver.internal.net.BoltServerAddress; + +public interface HostNameResolver +{ + Set resolve( BoltServerAddress initialRouter ); +} diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java index 395e55e629..5a9c4277fc 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java @@ -157,6 +157,6 @@ private static Rediscovery createRediscovery( BoltServerAddress initialRouter, R Clock clock, Logger log ) { ClusterCompositionProvider clusterComposition = new GetServersProcedureClusterCompositionProvider( clock, log ); - return new Rediscovery( initialRouter, settings, clock, log, clusterComposition ); + return new Rediscovery( initialRouter, settings, clock, log, clusterComposition, new DnsResolver( log ) ); } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java index 0d00a04621..baddf7abb9 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java @@ -18,6 +18,9 @@ */ package org.neo4j.driver.internal.cluster; +import java.util.HashSet; +import java.util.Set; + import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.spi.Connection; import org.neo4j.driver.internal.spi.ConnectionPool; @@ -37,15 +40,17 @@ public class Rediscovery private final Clock clock; private final Logger logger; private final ClusterCompositionProvider provider; + private final HostNameResolver hostNameResolver; public Rediscovery( BoltServerAddress initialRouter, RoutingSettings settings, Clock clock, Logger logger, - ClusterCompositionProvider provider ) + ClusterCompositionProvider provider, HostNameResolver hostNameResolver ) { this.initialRouter = initialRouter; this.settings = settings; this.clock = clock; this.logger = logger; this.provider = provider; + this.hostNameResolver = hostNameResolver; } // Given the current routing table and connection pool, use the connection composition provider to fetch a new @@ -76,8 +81,8 @@ public ClusterComposition lookupClusterComposition( ConnectionPool connections, private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPool connections, RoutingTable routingTable ) { - boolean triedInitialRouter = false; int size = routingTable.routerSize(); + Set triedServers = new HashSet<>(); for ( int i = 0; i < size; i++ ) { BoltServerAddress address = routingTable.nextRouter(); @@ -86,11 +91,21 @@ private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPoo break; } - if ( address.equals( initialRouter ) ) + ClusterComposition composition = lookupClusterCompositionOnRouter( address, connections, routingTable ); + if ( composition != null ) + { + return composition; + } + else { - triedInitialRouter = true; + triedServers.add( address ); } + } + Set ips = hostNameResolver.resolve( initialRouter ); + ips.removeAll( triedServers ); + for ( BoltServerAddress address : ips ) + { ClusterComposition composition = lookupClusterCompositionOnRouter( address, connections, routingTable ); if ( composition != null ) { @@ -98,11 +113,7 @@ private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPoo } } - if ( triedInitialRouter ) - { - return null; - } - return lookupClusterCompositionOnRouter( initialRouter, connections, routingTable ); + return null; } private ClusterComposition lookupClusterCompositionOnRouter( BoltServerAddress routerAddress, @@ -122,7 +133,7 @@ private ClusterComposition lookupClusterCompositionOnRouter( BoltServerAddress r { // connection turned out to be broken logger.error( format( "Failed to connect to routing server '%s'.", routerAddress ), t ); - routingTable.removeRouter( routerAddress ); + routingTable.forget( routerAddress ); return null; } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTable.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTable.java index 21ff292ddd..91a6d62808 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTable.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTable.java @@ -39,6 +39,4 @@ public interface RoutingTable int routerSize(); void removeWriter( BoltServerAddress toRemove ); - - void removeRouter( BoltServerAddress toRemove ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/net/BoltServerAddress.java b/driver/src/main/java/org/neo4j/driver/internal/net/BoltServerAddress.java index ed8a6764ff..a7be5657f7 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/net/BoltServerAddress.java +++ b/driver/src/main/java/org/neo4j/driver/internal/net/BoltServerAddress.java @@ -41,6 +41,12 @@ public static BoltServerAddress from( URI uri ) { port = DEFAULT_PORT; } + + if( uri.getHost() == null ) + { + throw new IllegalArgumentException( "Invalid URI format `" + uri.toString() + "`"); + } + return new BoltServerAddress( uri.getHost(), port ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java index 230ee2f712..3df69e3bbd 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java @@ -32,6 +32,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.neo4j.driver.internal.net.BoltServerAddress.LOCAL_DEFAULT; import static org.neo4j.driver.internal.util.Matchers.directDriverWithAddress; import static org.neo4j.driver.v1.Values.parameters; @@ -52,6 +53,38 @@ public void shouldUseDefaultPortIfMissing() assertThat( driver, is( directDriverWithAddress( LOCAL_DEFAULT ) ) ); } + @Test + public void shouldAllowIPv6Address() + { + // Given + URI uri = URI.create( "bolt://[::1]" ); + BoltServerAddress address = BoltServerAddress.from( uri ); + + // When + Driver driver = GraphDatabase.driver( uri ); + + // Then + assertThat( driver, is( directDriverWithAddress( address ) ) ); + } + + @Test + public void shouldRejectInvalidAddress() + { + // Given + URI uri = URI.create( "*" ); + + // When & Then + try + { + Driver driver = GraphDatabase.driver( uri ); + fail("Expecting error for wrong uri"); + } + catch( IllegalArgumentException e ) + { + assertThat( e.getMessage(), equalTo( "Invalid URI format `*`" ) ); + } + } + @Test public void shouldRegisterSingleServer() { diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/DnsResolverTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/DnsResolverTest.java new file mode 100644 index 0000000000..7f647ea8c4 --- /dev/null +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/DnsResolverTest.java @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal.cluster; + +import org.junit.Test; + +import java.net.UnknownHostException; +import java.util.Set; + +import org.neo4j.driver.internal.net.BoltServerAddress; +import org.neo4j.driver.v1.Logger; + +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +public class DnsResolverTest +{ + private DnsResolver resolver = new DnsResolver( mock( Logger.class ) ); + + @Test + public void shouldResolveDNSToIPs() + { + Set resolve = resolver.resolve( new BoltServerAddress( "google.com", 80 ) ); + assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) ); + } + + @Test + public void shouldResolveLocalhostIPDNSToIPs() + { + Set resolve = resolver.resolve( new BoltServerAddress( "127.0.0.1", 80 ) ); + assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) ); + } + + @Test + public void shouldResolveLocalhostDNSToIPs() + { + Set resolve = resolver.resolve( new BoltServerAddress( "localhost", 80 ) ); + assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) ); + } + + @Test + public void shouldResolveIPv6LocalhostDNSToIPs() + { + Set resolve = resolver.resolve( new BoltServerAddress( "[::1]", 80 ) ); + assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) ); + } + + @Test + public void shouldExceptionAndGiveDefaultValue() + { + Logger logger = mock( Logger.class ); + DnsResolver resolver = new DnsResolver( logger ); + Set resolve = resolver.resolve( new BoltServerAddress( "[/]", 80 ) ); + verify( logger ).error( any( String.class ), any( UnknownHostException.class ) ); + assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) ); + } +} diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java index 3bcd5bd104..db85973f98 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java @@ -26,7 +26,10 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.spi.Connection; @@ -66,6 +69,16 @@ @RunWith( Enclosed.class ) public class RediscoveryTest { + private static HostNameResolver directMapProvider = new HostNameResolver() + { + @Override + public Set resolve( BoltServerAddress initialRouter ) + { + Set directMap = new HashSet<>(); + directMap.add( initialRouter ); + return directMap; + } + }; private static ClusterCompositionResponse.Success success( ClusterComposition cluster ) { @@ -92,7 +105,7 @@ public void shouldTryConfiguredMaxRoutingFailures() throws Exception when( mockedProvider.getClusterComposition( any( Connection.class ) ) ) .thenReturn( success( INVALID_CLUSTER_COMPOSITION ) ); - Rediscovery rediscovery = new Rediscovery( A, settings, clock, DEV_NULL_LOGGER, mockedProvider ); + Rediscovery rediscovery = new Rediscovery( A, settings, clock, DEV_NULL_LOGGER, mockedProvider, directMapProvider ); // when try @@ -372,7 +385,6 @@ public void shouldUseInitialRouterWhenNoneOfExistingRoutersRespond() .thenThrow( new ServiceUnavailableException( "Can't connect" ) ); RoutingTable routingTable = new TestRoutingTable( B, C ); - ClusterComposition composition = rediscover( A, connections, routingTable, clusterComposition ); assertEquals( VALID_CLUSTER_COMPOSITION, composition ); @@ -454,7 +466,8 @@ private static ClusterComposition rediscover( BoltServerAddress initialRouter, C Clock mockedClock = mock( Clock.class ); Logger mockedLogger = mock( Logger.class ); - Rediscovery rediscovery = new Rediscovery( initialRouter, settings, mockedClock, mockedLogger, provider ); + Rediscovery rediscovery = new Rediscovery( initialRouter, settings, mockedClock, mockedLogger, provider, + directMapProvider ); return rediscovery.lookupClusterComposition( connections, routingTable ); } @@ -468,9 +481,9 @@ private static class TestRoutingTable extends ClusterRoutingTable } @Override - public void removeRouter( BoltServerAddress router ) + public void forget( BoltServerAddress router ) { - super.removeRouter( router ); + super.forget( router ); removedRouters.add( router ); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java index 6121d69771..2dc406a18d 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java @@ -225,7 +225,7 @@ private void verifyServiceUnavailableHandling( Connection connection, RoutingTab assertThat( e.getCause(), instanceOf( ServiceUnavailableException.class ) ); BoltServerAddress address = connection.boltServerAddress(); - assertThat( routingTable, containsRouter( address ) ); + assertThat( routingTable, not( containsRouter( address ) ) ); assertThat( routingTable, not( containsReader( address ) ) ); assertThat( routingTable, not( containsWriter( address ) ) ); assertFalse( connectionPool.hasAddress( address ) ); diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/BookmarkIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/BookmarkIT.java index 7957952a67..2ccef793de 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/BookmarkIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/BookmarkIT.java @@ -27,7 +27,9 @@ import org.neo4j.driver.v1.AccessMode; import org.neo4j.driver.v1.Driver; +import org.neo4j.driver.v1.GraphDatabase; import org.neo4j.driver.v1.Session; +import org.neo4j.driver.v1.StatementResult; import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.TransientException; @@ -35,6 +37,7 @@ import org.neo4j.driver.v1.util.TestNeo4jSession; import static java.util.Arrays.asList; +import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; @@ -66,6 +69,21 @@ public void assumeBookmarkSupport() serverVersion.greaterThanOrEqual( v3_1_0 ) ); } + @Test + public void shouldConnectIPv6Uri() + { + // Given + try( Driver driver = GraphDatabase.driver( "bolt://[::1]:7687" ); + Session session = driver.session() ) + { + // When + StatementResult result = session.run( "RETURN 1" ); + + // Then + assertThat( result.single().get( 0 ).asInt(), equalTo( 1 ) ); + } + } + @Test public void shouldReceiveBookmarkOnSuccessfulCommit() throws Throwable { diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/TLSSocketChannelIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/TLSSocketChannelIT.java index fe84405a42..089d16dbd9 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/TLSSocketChannelIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/TLSSocketChannelIT.java @@ -231,8 +231,7 @@ private void createFakeServerCertPairInKnownCerts( BoltServerAddress address, Fi public void shouldFailTLSHandshakeDueToServerCertNotSignedByKnownCA() throws Throwable { // Given - neo4j.restart( - Neo4jSettings.TEST_SETTINGS.updateWith( + neo4j.restart( Neo4jSettings.TEST_SETTINGS.updateWith( Neo4jSettings.CERT_DIR, folder.getRoot().getAbsolutePath().replace( "\\", "/" ) ) ); SocketChannel channel = SocketChannel.open(); diff --git a/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jSettings.java b/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jSettings.java index 199cf2829f..f808f8a4e9 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jSettings.java +++ b/driver/src/test/java/org/neo4j/driver/v1/util/Neo4jSettings.java @@ -32,6 +32,8 @@ public class Neo4jSettings public static final String DATA_DIR = "dbms.directories.data"; public static final String CERT_DIR = "dbms.directories.certificates"; public static final String IMPORT_DIR = "dbms.directories.import"; + public static final String LISTEN_ADDR = "dbms.connectors.default_listen_address"; // only valid for 3.1+ + public static final String IPV6_ENABLED_ADDR = "::"; private static final String DEFAULT_IMPORT_DIR = "import"; private static final String DEFAULT_CERT_DIR = "certificates"; @@ -48,7 +50,8 @@ public class Neo4jSettings CERT_DIR, DEFAULT_CERT_DIR, DATA_DIR, DEFAULT_DATA_DIR, IMPORT_DIR, DEFAULT_IMPORT_DIR, - AUTH_ENABLED, "false" ), Collections.emptySet() ); + AUTH_ENABLED, "false", + LISTEN_ADDR, IPV6_ENABLED_ADDR ), Collections.emptySet() ); private Neo4jSettings( Map settings, Set excludes ) {