Skip to content

Commit

Permalink
Bolt rejects nodes, rels, paths as parameters
Browse files Browse the repository at this point in the history 
When encountering a node, relationship or path as parameter Bolt now throws an exception.
  • Loading branch information
@pontusmelke
pontusmelke committed Feb 9, 2016
1 parent 0369161 commit e6fb514
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 100 deletions.
40 changes: 20 additions & 20 deletions community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java
View file
Expand Up @@ -211,6 +211,7 @@ public void packRawMap( Map<String, Object> map ) throws IOException
}
}
}

// TODO: combine these
public void packProperties( PropertyContainer entity ) throws IOException
{
Expand All @@ -224,7 +225,6 @@ public void packProperties( PropertyContainer entity ) throws IOException
}
}


public static class Unpacker extends PackStream.Unpacker
{
private PathPack.Unpacker pathUnpacker = new PathPack.Unpacker();
Expand Down Expand Up @@ -265,25 +265,25 @@ public Object unpack() throws IOException
char signature = unpackStructSignature();
switch ( signature )
{
case NODE:
{
return ValueNode.unpackFields( this );
}
case RELATIONSHIP:
{
return ValueRelationship.unpackFields( this );
}
case UNBOUND_RELATIONSHIP:
{
return ValueUnboundRelationship.unpackFields( this );
}
case PATH:
{
return pathUnpacker.unpackFields( this );
}
default:
throw new BoltIOException( Status.Request.InvalidFormat,
"Unknown struct type: " + Integer.toHexString(signature) );
case NODE:
{
throw new BoltIOException( Status.Request.Invalid, "Nodes cannot be unpacked." );
}
case RELATIONSHIP:
{
throw new BoltIOException( Status.Request.Invalid, "Relationships cannot be unpacked." );
}
case UNBOUND_RELATIONSHIP:
{
throw new BoltIOException( Status.Request.Invalid, "Relationships cannot be unpacked." );
}
case PATH:
{
throw new BoltIOException( Status.Request.Invalid, "Paths cannot be unpacked." );
}
default:
throw new BoltIOException( Status.Request.InvalidFormat,
"Unknown struct type: " + Integer.toHexString( signature ) );
}
}
case END_OF_STREAM:
Expand Down
2 changes: 1 addition & 1 deletion community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/PackStreamMessageFormatV1.java
View file
Expand Up @@ -163,7 +163,7 @@ public void handleFailureMessage( Status status, String message )

packer.pack( "message" );
packer.pack( message );

onMessageComplete.onMessageComplete();
}

Expand Down
11 changes: 3 additions & 8 deletions community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/internal/CypherAdapterStream.java
View file
Expand Up @@ -61,14 +61,9 @@ public String[] fieldNames()
@Override
public void accept( final Visitor visitor ) throws Exception
{
delegate.accept( new Result.ResultVisitor<Exception>()
{
@Override
public boolean visit( Result.ResultRow row ) throws Exception
{
visitor.visit( currentRecord.reset( row ) );
return true;
}
delegate.accept( row -> {
visitor.visit( currentRecord.reset( row ) );
return true;
} );

QueryExecutionType qt = delegate.getQueryExecutionType();
Expand Down
66 changes: 48 additions & 18 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/docs/BoltValueStructsDocTest.java
View file
Expand Up @@ -31,11 +31,9 @@

import org.neo4j.bolt.v1.messaging.Neo4jPack;
import org.neo4j.bolt.v1.messaging.infrastructure.ValueNode;
import org.neo4j.bolt.v1.messaging.infrastructure.ValuePath;
import org.neo4j.bolt.v1.messaging.infrastructure.ValueRelationship;
import org.neo4j.bolt.v1.messaging.infrastructure.ValueUnboundRelationship;
import org.neo4j.bolt.v1.packstream.PackedInputArray;
import org.neo4j.bolt.v1.packstream.PackedOutputArray;
import org.neo4j.kernel.impl.util.HexPrinter;

import static org.junit.Assert.assertEquals;
import static org.neo4j.bolt.v1.messaging.example.Nodes.ALICE;
Expand All @@ -50,14 +48,54 @@ public class BoltValueStructsDocTest
{

// This lookup is used to check we hydrate structures to the correct types
public static Map<String, Class> expectedClass = new HashMap<>();
public static Map<String, String> expectedSerialization = new HashMap<>();

static
{
expectedClass.put( "Node", ValueNode.class );
expectedClass.put( "Relationship", ValueRelationship.class );
expectedClass.put( "UnboundRelationship", ValueUnboundRelationship.class );
expectedClass.put( "Path", ValuePath.class );
expectedSerialization.put( "Node", "B3 4E C9 05\n" +
"39 92 86 42\n" +
"61 6E 61 6E\n" +
"61 86 50 65\n" +
"72 73 6F 6E\n" +
"A1 81 6B C9\n" +
"30 39" );
expectedSerialization.put( "Relationship", "B5 52 C9 05\n" +
"39 C9 05 39\n" +
"C9 05 39 85\n" +
"31 32 33 34\n" +
"35 A1 81 6B\n" +
"C9 30 39" );
expectedSerialization.put( "UnboundRelationship", "B3 72 C9 05\n" +
"39 87 20 22\n" +
"4B 4E 4F 57\n" +
"53 A1 81 6B\n" +
"C9 30 39" );
expectedSerialization.put( "Path", "B3 50 92 B3\n" +
"4E C9 03 E9\n" +
"92 86 50 65\n" +
"72 73 6F 6E\n" +
"88 45 6D 70\n" +
"6C 6F 79 65\n" +
"65 A2 84 6E\n" +
"61 6D 65 85\n" +
"41 6C 69 63\n" +
"65 83 61 67\n" +
"65 21 B3 4E\n" +
"C9 03 EA 92\n" +
"86 50 65 72\n" +
"73 6F 6E 88\n" +
"45 6D 70 6C\n" +
"6F 79 65 65\n" +
"A2 84 6E 61\n" +
"6D 65 83 42\n" +
"6F 62 83 61\n" +
"67 65 2C 91\n" +
"B3 72 0C 85\n" +
"4B 4E 4F 57\n" +
"53 A1 85 73\n" +
"69 6E 63 65\n" +
"C9 07 CF 92\n" +
"01 01" );
}

@Parameterized.Parameter(0)
Expand Down Expand Up @@ -97,18 +135,10 @@ public void ensureSerializingMessagesAsDocumentedWorks() throws Throwable

// Then unpack again into a regular object
byte[] bytes = output.bytes();
//System.out.println("Serialized bytes:\n" + HexPrinter.hex( bytes ));
PackedInputArray input = new PackedInputArray( bytes );
Neo4jPack.Unpacker unpacker = new Neo4jPack.Unpacker( input );
Object unpacked = unpacker.unpack();
String hex = HexPrinter.hex( bytes, 4, "\n" );

// Then it should get interpreted as the documented structure
assertEquals( expectedClass.get( struct.name() ), unpacked.getClass() );

// Hello, future traveler. The assertion above is not strictly necessary. What we're trying
// to do here is simply to ensure that the documented structure is what we get back out
// when we deserialize, the name of the class does not strictly have to map to the name in
// the docs, if that is causing you trouble.
assertEquals( expectedSerialization.get( struct.name() ), hex );
}

private void packValueOf( DocStruct.Field field, Neo4jPack.Packer packer ) throws IOException
Expand Down
117 changes: 108 additions & 9 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/MessageFormatTest.java
View file
Expand Up @@ -21,13 +21,18 @@

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import org.neo4j.bolt.v1.messaging.message.ResetMessage;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.Path;
import org.neo4j.graphdb.Relationship;
import org.neo4j.graphdb.RelationshipType;
import org.neo4j.kernel.api.exceptions.Status;
import org.neo4j.kernel.impl.util.HexPrinter;
Expand All @@ -50,6 +55,17 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;

import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_LENGTH_ONE;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_LENGTH_TWO;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_LENGTH_ZERO;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_LOOP;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_NODES_VISITED_MULTIPLE_TIMES;
import static org.neo4j.bolt.v1.messaging.example.Paths.PATH_WITH_RELATIONSHIP_TRAVERSED_AGAINST_ITS_DIRECTION;
import static org.neo4j.bolt.v1.messaging.example.Paths
.PATH_WITH_RELATIONSHIP_TRAVERSED_MULTIPLE_TIMES_IN_SAME_DIRECTION;
import static org.neo4j.bolt.v1.messaging.util.MessageMatchers.serialize;
import static org.neo4j.graphdb.Label.label;
import static org.neo4j.helpers.collection.MapUtil.map;
import static org.neo4j.bolt.v1.messaging.PackStreamMessageFormatV1.Writer.NO_OP;
Expand All @@ -58,6 +74,9 @@

public class MessageFormatTest
{
@Rule
public ExpectedException exception = ExpectedException.none();

@Test
public void shouldHandleCommonMessages() throws Throwable
{
Expand Down Expand Up @@ -113,7 +132,7 @@ public void shouldSerializeBasicTypes() throws Throwable
assertSerializesNeoValue( map( "k", null ) );
assertSerializesNeoValue( map( "k", true ) );
assertSerializesNeoValue( map( "k", false ) );
assertSerializesNeoValue( map( "k", 1337l ) );
assertSerializesNeoValue( map( "k", 1337L ) );
assertSerializesNeoValue( map( "k", 133.7d ) );
assertSerializesNeoValue( map( "k", "Hello" ) );
assertSerializesNeoValue( map( "k", asList( "one", "", "three" ) ) );
Expand All @@ -122,24 +141,104 @@ public void shouldSerializeBasicTypes() throws Throwable
@Test
public void shouldSerializeNode() throws Throwable
{
assertSerializesNeoValue( new ValueNode( 12l, asList( label( "User" ), label( "Banana" ) ),
map( "name", "Bob", "age", 14 ) ) );
ValueNode valueNode = new ValueNode( 12L, asList( label( "User" ), label( "Banana" ) ),
map( "name", "Bob", "age", 14 ) );

assertThat( serialized( valueNode ),
equalTo( "B1 71 91 B3 4E 0C 92 84 55 73 65 72 86 42 61 6E\n" +
"61 6E 61 A2 84 6E 61 6D 65 83 42 6F 62 83 61 67\n" +
"65 0E" ) );
}

@Test
public void shouldSerializeRelationship() throws Throwable
{
assertSerializesNeoValue( new ValueRelationship( 12l, 1l, 2l, RelationshipType.withName( "KNOWS" ),
map( "name", "Bob", "age", 14 ) ) );
ValueRelationship valueRelationship = new ValueRelationship( 12L, 1L, 2L, RelationshipType.withName( "KNOWS" ),
map( "name", "Bob", "age", 14 ) );

assertThat( serialized( valueRelationship ),
equalTo( "B1 71 91 B5 52 0C 01 02 85 4B 4E 4F 57 53 A2 84\n" +
"6E 61 6D 65 83 42 6F 62 83 61 67 65 0E" ) );
}

@Test
public void shouldSerializePaths() throws Throwable
{
for ( Path path : ALL_PATHS )
{
assertSerializesNeoValue( path );
}
assertThat( serialized( PATH_WITH_LENGTH_ZERO ),
equalTo( "B1 71 91 B3 50 91 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 90 90" ) );
assertThat( serialized( PATH_WITH_LENGTH_ONE ),
equalTo( "B1 71 91 B3 50 92 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 B3 4E C9\n" +
"03 EA 92 86 50 65 72 73 6F 6E 88 45 6D 70 6C 6F\n" +
"79 65 65 A2 84 6E 61 6D 65 83 42 6F 62 83 61 67\n" +
"65 2C 91 B3 72 0C 85 4B 4E 4F 57 53 A1 85 73 69\n" +
"6E 63 65 C9 07 CF 92 01 01"
) );
assertThat( serialized( PATH_WITH_LENGTH_TWO ),
equalTo( "B1 71 91 B3 50 93 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 B3 4E C9\n" +
"03 EB 91 86 50 65 72 73 6F 6E A1 84 6E 61 6D 65\n" +
"85 43 61 72 6F 6C B3 4E C9 03 EC 90 A1 84 6E 61\n" +
"6D 65 84 44 61 76 65 92 B3 72 0D 85 4C 49 4B 45\n" +
"53 A0 B3 72 22 8A 4D 41 52 52 49 45 44 5F 54 4F\n" +
"A0 94 01 01 02 02" ) );
assertThat( serialized( PATH_WITH_RELATIONSHIP_TRAVERSED_AGAINST_ITS_DIRECTION),
equalTo( "B1 71 91 B3 50 94 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 B3 4E C9\n" +
"03 EA 92 86 50 65 72 73 6F 6E 88 45 6D 70 6C 6F\n" +
"79 65 65 A2 84 6E 61 6D 65 83 42 6F 62 83 61 67\n" +
"65 2C B3 4E C9 03 EB 91 86 50 65 72 73 6F 6E A1\n" +
"84 6E 61 6D 65 85 43 61 72 6F 6C B3 4E C9 03 EC\n" +
"90 A1 84 6E 61 6D 65 84 44 61 76 65 93 B3 72 0C\n" +
"85 4B 4E 4F 57 53 A1 85 73 69 6E 63 65 C9 07 CF\n" +
"B3 72 20 88 44 49 53 4C 49 4B 45 53 A0 B3 72 22\n" +
"8A 4D 41 52 52 49 45 44 5F 54 4F A0 96 01 01 FE\n" +
"02 03 03" ) );
assertThat( serialized( PATH_WITH_NODES_VISITED_MULTIPLE_TIMES ),
equalTo( "B1 71 91 B3 50 93 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 B3 4E C9\n" +
"03 EA 92 86 50 65 72 73 6F 6E 88 45 6D 70 6C 6F\n" +
"79 65 65 A2 84 6E 61 6D 65 83 42 6F 62 83 61 67\n" +
"65 2C B3 4E C9 03 EB 91 86 50 65 72 73 6F 6E A1\n" +
"84 6E 61 6D 65 85 43 61 72 6F 6C 93 B3 72 0C 85\n" +
"4B 4E 4F 57 53 A1 85 73 69 6E 63 65 C9 07 CF B3\n" +
"72 0D 85 4C 49 4B 45 53 A0 B3 72 20 88 44 49 53\n" +
"4C 49 4B 45 53 A0 9A 01 01 FF 00 02 02 03 01 FD\n" +
"02" ) );
assertThat( serialized( PATH_WITH_RELATIONSHIP_TRAVERSED_MULTIPLE_TIMES_IN_SAME_DIRECTION ),
equalTo( "B1 71 91 B3 50 94 B3 4E C9 03 E9 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 85 41 6C 69 63 65 83 61 67 65 21 B3 4E C9\n" +
"03 EB 91 86 50 65 72 73 6F 6E A1 84 6E 61 6D 65\n" +
"85 43 61 72 6F 6C B3 4E C9 03 EA 92 86 50 65 72\n" +
"73 6F 6E 88 45 6D 70 6C 6F 79 65 65 A2 84 6E 61\n" +
"6D 65 83 42 6F 62 83 61 67 65 2C B3 4E C9 03 EC\n" +
"90 A1 84 6E 61 6D 65 84 44 61 76 65 94 B3 72 0D\n" +
"85 4C 49 4B 45 53 A0 B3 72 20 88 44 49 53 4C 49\n" +
"4B 45 53 A0 B3 72 0C 85 4B 4E 4F 57 53 A1 85 73\n" +
"69 6E 63 65 C9 07 CF B3 72 22 8A 4D 41 52 52 49\n" +
"45 44 5F 54 4F A0 9A 01 01 02 02 FD 00 01 01 04\n" +
"03") );
assertThat( serialized( PATH_WITH_LOOP ),
equalTo( "B1 71 91 B3 50 92 B3 4E C9 03 EB 91 86 50 65 72\n" +
"73 6F 6E A1 84 6E 61 6D 65 85 43 61 72 6F 6C B3\n" +
"4E C9 03 EC 90 A1 84 6E 61 6D 65 84 44 61 76 65\n" +
"92 B3 72 22 8A 4D 41 52 52 49 45 44 5F 54 4F A0\n" +
"B3 72 2C 89 57 4F 52 4B 53 5F 46 4F 52 A0 94 01\n" +
"01 02 01") );
}

private String serialized( Object object ) throws IOException
{
RecordMessage message =
new RecordMessage( record( object ) );
return HexPrinter.hex( serialize( message ), 4, " " );
}

private void assertSerializes( Message msg ) throws IOException
Expand Down

0 comments on commit e6fb514

Please sign in to comment.