Skip to content

Commit

Permalink
Making Bolt advertised address affined to HTTP host
Browse files Browse the repository at this point in the history
This is a belt-n-braces style commit that tries to address some of the
inconveniences of Bolt server being a kernel extension.
If no sane advertised address is specified for Bolt, then the discovery
service will now try to figure one out using the HTTP Host header
as the basis for computing a Bolt connection URI.

This embeds some assumptions around advertised addresses that may
not be true in all possible cases but should be true for the vast majority.
In particular if the server is bound to a different NIC for HTTP and Bolt,
it's likely this won't work as expected.
  • Loading branch information
jimwebber committed Nov 10, 2016
1 parent a7b830b commit ea9204c
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 22 deletions.
Expand Up @@ -147,7 +147,7 @@ public List<Arguments> possibleArguments()
public String description()
{
return "Import a collection of CSV files with --mode=csv (default), or a database from " +
"a pre-3.0 installation with --mode=database.";
"a pre-3.0 installation with --mode=database.";
}

@Override
Expand Down
Expand Up @@ -31,7 +31,7 @@ public class SocketAddress
private final String hostname;
private final int port;

SocketAddress( String hostname, int port )
public SocketAddress( String hostname, int port )
{
this.hostname = hostname;
this.port = port;
Expand Down Expand Up @@ -73,8 +73,7 @@ public boolean equals( Object o )
return false;
}
SocketAddress that = (SocketAddress) o;
return port == that.port &&
Objects.equals( hostname, that.hostname );
return port == that.port && Objects.equals( hostname, that.hostname );
}

@Override
Expand Down
Expand Up @@ -33,7 +33,7 @@ public void shouldCreateAdvertisedSocketAddressWithLeadingWhitespace() throws Ex
String addressString = whitespace( 1 ) + "localhost:9999";

// when
AdvertisedSocketAddress address = SocketAddressFormat.socketAddress( addressString, AdvertisedSocketAddress::new );
SocketAddress address = SocketAddressFormat.socketAddress( addressString, SocketAddress::new );

// then
assertEquals( "localhost", address.getHostname() );
Expand All @@ -47,7 +47,7 @@ public void shouldCreateAdvertisedSocketAddressWithTrailingWhitespace() throws E
String addressString = "localhost:9999" + whitespace( 1 );

// when
AdvertisedSocketAddress address = SocketAddressFormat.socketAddress( addressString, AdvertisedSocketAddress::new );
SocketAddress address = SocketAddressFormat.socketAddress( addressString, SocketAddress::new );

// then
assertEquals( "localhost", address.getHostname() );
Expand All @@ -60,7 +60,7 @@ public void shouldFailToCreateSocketAddressWithMixedInWhitespace()
String addressString = "localhost" + whitespace( 1 ) + ":9999";
try
{
SocketAddressFormat.socketAddress( addressString, AdvertisedSocketAddress::new );
SocketAddressFormat.socketAddress( addressString, SocketAddress::new );
fail( "Should have thrown an exception" );
}
catch ( IllegalArgumentException e )
Expand All @@ -75,7 +75,7 @@ public void shouldFailToCreateSocketWithTrailingNonNumbers()
String addressString = "localhost:9999abc";
try
{
SocketAddressFormat.socketAddress( addressString, AdvertisedSocketAddress::new );
SocketAddressFormat.socketAddress( addressString, SocketAddress::new );
fail( "Should have thrown an exception" );
}
catch ( IllegalArgumentException e )
Expand All @@ -90,7 +90,7 @@ public void shouldFailOnMissingPort()
String addressString = "localhost:";
try
{
SocketAddressFormat.socketAddress( addressString, AdvertisedSocketAddress::new );
SocketAddressFormat.socketAddress( addressString, SocketAddress::new );
fail( "Should have thrown an exception" );
}
catch ( IllegalArgumentException e )
Expand Down
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.server.rest.discovery;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.Optional;
import javax.ws.rs.GET;
Expand All @@ -28,6 +29,7 @@
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;

import org.neo4j.helpers.AdvertisedSocketAddress;
import org.neo4j.kernel.configuration.Config;
Expand Down Expand Up @@ -56,7 +58,7 @@ public DiscoveryService( @Context Config config, @Context OutputFormat outputFor

@GET
@Produces( MediaType.APPLICATION_JSON )
public Response getDiscoveryDocument( @HeaderParam( "host" ) String host ) throws URISyntaxException
public Response getDiscoveryDocument( @Context UriInfo uriInfo ) throws URISyntaxException
{
String managementUri = config.get( ServerSettings.management_api_path ).getPath() + "/";
String dataUri = config.get( ServerSettings.rest_api_path ).getPath() + "/";
Expand All @@ -71,7 +73,8 @@ public Response getDiscoveryDocument( @HeaderParam( "host" ) String host ) throw
{
// Use the port specified in the config, but not the host
return outputFormat.ok( new DiscoveryRepresentation( managementUri, dataUri,
fabricateBoltAddress( host, advertisedSocketAddress.getPort() ) ) );
new AdvertisedSocketAddress( uriInfo.getBaseUri().getHost(),
advertisedSocketAddress.getPort() ) ) );
}
else
{
Expand All @@ -84,16 +87,11 @@ public Response getDiscoveryDocument( @HeaderParam( "host" ) String host ) throw
else
{
// There's no config, compute possible endpoint using host header and default bolt port.
return outputFormat
.ok( new DiscoveryRepresentation( managementUri, dataUri, fabricateBoltAddress( host, 7687 ) ) );
return outputFormat.ok( new DiscoveryRepresentation( managementUri, dataUri,
new AdvertisedSocketAddress( uriInfo.getBaseUri().getHost(), 7687 ) ) );
}
}

private AdvertisedSocketAddress fabricateBoltAddress( String host, int port )
{
return new AdvertisedSocketAddress( "bolt://" + host, port );
}

@GET
@Produces( MediaType.WILDCARD )
public Response redirectToBrowser()
Expand Down
2 changes: 2 additions & 0 deletions community/server/src/test/java/org/neo4j/server/BoltIT.java
Expand Up @@ -37,6 +37,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertFalse;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.boltConnector;
import static org.neo4j.server.helpers.CommunityServerBuilder.server;

Expand Down Expand Up @@ -95,6 +96,7 @@ public void boltAddressShouldAppearToComeFromTheSameOriginAsTheHttpAddressEvenTh
// Then
Map<String,Object> map = JsonHelper.jsonToMap( response.getEntity() );
assertThat( String.valueOf( map.get( "bolt" ) ), containsString( "bolt://" + host ) );
assertFalse( String.valueOf( map.get( "bolt" ) ).contains( "bolt://bolt://" ) );
}

@Test
Expand Down
Expand Up @@ -26,6 +26,7 @@
import java.net.URISyntaxException;
import java.util.HashMap;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;

import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.AdvertisedSocketAddress;
Expand Down Expand Up @@ -85,7 +86,7 @@ private DiscoveryService testDiscoveryService() throws URISyntaxException
@Test
public void shouldReturnValidJSON() throws Exception
{
Response response = testDiscoveryService().getDiscoveryDocument( "localhost" );
Response response = testDiscoveryService().getDiscoveryDocument( uriInfo( "localhost" ) );
String json = new String( (byte[]) response.getEntity() );

assertNotNull( json );
Expand All @@ -94,26 +95,34 @@ public void shouldReturnValidJSON() throws Exception
assertThat( json, is( not( "null" ) ) );
}

private UriInfo uriInfo( String host )
{
URI uri = URI.create( host );
UriInfo uriInfo = mock( UriInfo.class );
when( uriInfo.getBaseUri() ).thenReturn( uri );
return uriInfo;
}

@Test
public void shouldReturnBoltURI() throws Exception
{
Response response = testDiscoveryService().getDiscoveryDocument( "localhost" );
Response response = testDiscoveryService().getDiscoveryDocument( uriInfo( "localhost" ) );
String json = new String( (byte[]) response.getEntity() );
assertThat( json, containsString( "\"bolt\" : \"bolt://" + boltAddress ) );
}

@Test
public void shouldReturnDataURI() throws Exception
{
Response response = testDiscoveryService().getDiscoveryDocument( "localhost" );
Response response = testDiscoveryService().getDiscoveryDocument( uriInfo( "localhost" ) );
String json = new String( (byte[]) response.getEntity() );
assertThat( json, containsString( "\"data\" : \"" + baseUri + dataUri + "/\"" ) );
}

@Test
public void shouldReturnManagementURI() throws Exception
{
Response response = testDiscoveryService().getDiscoveryDocument( "localhost" );
Response response = testDiscoveryService().getDiscoveryDocument( uriInfo( "localhost" ) );
String json = new String( (byte[]) response.getEntity() );
assertThat( json, containsString( "\"management\" : \"" + baseUri + managementUri + "/\"" ) );
}
Expand Down

0 comments on commit ea9204c

Please sign in to comment.