Skip to content

Commit 5150775

Browse files
author
Zhen Li
committed
Adding more checks around the allowed input parameter types
The input parameters could be of type: * Map<String, Object> * Record * MapValue generated via Values#parameters * MapValue If the input parameters are provided by a Map<String, Object> or Record, or Values#parameters, then any unsupported types will be captured via Values#value. If the input parameters are provided by a MapValue directly, then any unsupported types will be captured at PackStream#pack.
1 parent 13a4cfd commit 5150775

File tree

10 files changed

+362
-179
lines changed

10 files changed

+362
-179
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV1.java‎

Lines changed: 1 addition & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.neo4j.driver.internal.value.RelationshipValue;
4444
import org.neo4j.driver.v1.Value;
4545
import org.neo4j.driver.v1.exceptions.ClientException;
46-
import org.neo4j.driver.v1.types.Entity;
4746
import org.neo4j.driver.v1.types.Node;
4847
import org.neo4j.driver.v1.types.Path;
4948
import org.neo4j.driver.v1.types.Relationship;
@@ -252,79 +251,8 @@ void packInternalValue( InternalValue value ) throws IOException
252251
}
253252
break;
254253

255-
case NODE:
256-
{
257-
Node node = value.asNode();
258-
packNode( node );
259-
}
260-
break;
261-
262-
case RELATIONSHIP:
263-
{
264-
Relationship rel = value.asRelationship();
265-
packer.packStructHeader( 5, RELATIONSHIP );
266-
packer.pack( rel.id() );
267-
packer.pack( rel.startNodeId() );
268-
packer.pack( rel.endNodeId() );
269-
270-
packer.pack( rel.type() );
271-
272-
packProperties( rel );
273-
}
274-
break;
275-
276-
case PATH:
277-
Path path = value.asPath();
278-
packer.packStructHeader( 3, PATH );
279-
280-
// Unique nodes
281-
Map<Node,Integer> nodeIdx = Iterables.newLinkedHashMapWithSize( path.length() + 1 );
282-
for ( Node node : path.nodes() )
283-
{
284-
if ( !nodeIdx.containsKey( node ) )
285-
{
286-
nodeIdx.put( node, nodeIdx.size() );
287-
}
288-
}
289-
packer.packListHeader( nodeIdx.size() );
290-
for ( Node node : nodeIdx.keySet() )
291-
{
292-
packNode( node );
293-
}
294-
295-
// Unique rels
296-
Map<Relationship,Integer> relIdx = Iterables.newLinkedHashMapWithSize( path.length() );
297-
for ( Relationship rel : path.relationships() )
298-
{
299-
if ( !relIdx.containsKey( rel ) )
300-
{
301-
relIdx.put( rel, relIdx.size() + 1 );
302-
}
303-
}
304-
packer.packListHeader( relIdx.size() );
305-
for ( Relationship rel : relIdx.keySet() )
306-
{
307-
packer.packStructHeader( 3, UNBOUND_RELATIONSHIP );
308-
packer.pack( rel.id() );
309-
packer.pack( rel.type() );
310-
packProperties( rel );
311-
}
312-
313-
// Sequence
314-
packer.packListHeader( path.length() * 2 );
315-
for ( Path.Segment seg : path )
316-
{
317-
Relationship rel = seg.relationship();
318-
long relEndId = rel.endNodeId();
319-
long segEndId = seg.end().id();
320-
int size = relEndId == segEndId ? relIdx.get( rel ) : -relIdx.get( rel );
321-
packer.pack( size );
322-
packer.pack( nodeIdx.get( seg.end() ) );
323-
}
324-
break;
325-
326254
default:
327-
throw new IOException( "Unknown type: " + value );
255+
throw new IOException( "Unknown type: " + value.type().name() );
328256
}
329257
}
330258

@@ -333,32 +261,6 @@ public void write( Message msg ) throws IOException
333261
{
334262
msg.dispatch( this );
335263
}
336-
337-
private void packNode( Node node ) throws IOException
338-
{
339-
packer.packStructHeader( NODE_FIELDS, NODE );
340-
packer.pack( node.id() );
341-
342-
Iterable<String> labels = node.labels();
343-
packer.packListHeader( Iterables.count( labels ) );
344-
for ( String label : labels )
345-
{
346-
packer.pack( label );
347-
}
348-
349-
packProperties( node );
350-
}
351-
352-
private void packProperties( Entity entity ) throws IOException
353-
{
354-
Iterable<String> keys = entity.keys();
355-
packer.packMapHeader( entity.size() );
356-
for ( String propKey : keys )
357-
{
358-
packer.pack( propKey );
359-
packValue( entity.get( propKey ) );
360-
}
361-
}
362264
}
363265

364266
static class ReaderV1 implements MessageFormat.Reader

‎driver/src/main/java/org/neo4j/driver/v1/Statement.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import java.util.Map;
2222

23+
import org.neo4j.driver.internal.value.MapValue;
24+
import org.neo4j.driver.v1.exceptions.ClientException;
2325
import org.neo4j.driver.v1.summary.ResultSummary;
2426
import org.neo4j.driver.v1.util.Immutable;
2527

@@ -52,7 +54,18 @@ public class Statement
5254
public Statement( String text, Value parameters )
5355
{
5456
this.text = text;
55-
this.parameters = parameters == null ? Values.EmptyMap : parameters;
57+
if( parameters == null )
58+
{
59+
this.parameters = Values.EmptyMap;
60+
}
61+
else if ( parameters instanceof MapValue )
62+
{
63+
this.parameters = parameters;
64+
}
65+
else
66+
{
67+
throw new ClientException( "The parameters should be provided as Map type. Unsupported parameters type: " + parameters.type().name() );
68+
}
5669
}
5770

5871
/**

‎driver/src/main/java/org/neo4j/driver/v1/Values.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,7 @@ public static Value value( Object value )
9494
{
9595
if ( value == null ) { return NullValue.NULL; }
9696

97-
if ( value instanceof NodeValue || value instanceof RelationshipValue || value instanceof PathValue )
98-
{
99-
throw new ClientException( "Unsupported param type " + ((AsValue) value).asValue().type().name() );
100-
}
97+
assertParameter( value );
10198
if ( value instanceof AsValue ) { return ((AsValue) value).asValue(); }
10299
if ( value instanceof Boolean ) { return value( (boolean) value ); }
103100
if ( value instanceof String ) { return value( (String) value ); }
@@ -388,7 +385,6 @@ public static Value parameters( Object... keysAndValues )
388385
for ( int i = 0; i < keysAndValues.length; i += 2 )
389386
{
390387
Object value = keysAndValues[i + 1];
391-
assertParameter( value );
392388
map.put( keysAndValues[i].toString(), value( value ) );
393389
}
394390
return value( map );
@@ -673,15 +669,15 @@ public static <T> Function<Value,List<T>> ofList( final Function<Value,T> innerM
673669

674670
private static void assertParameter( Object value )
675671
{
676-
if ( value instanceof Node )
672+
if ( value instanceof Node || value instanceof NodeValue )
677673
{
678674
throw new ClientException( "Nodes can't be used as parameters." );
679675
}
680-
if ( value instanceof Relationship )
676+
if ( value instanceof Relationship || value instanceof RelationshipValue )
681677
{
682678
throw new ClientException( "Relationships can't be used as parameters." );
683679
}
684-
if ( value instanceof Path )
680+
if ( value instanceof Path || value instanceof PathValue )
685681
{
686682
throw new ClientException( "Paths can't be used as parameters." );
687683
}

‎driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@
6363
import static org.junit.Assert.assertEquals;
6464
import static org.junit.Assert.assertNotEquals;
6565
import static org.junit.Assert.assertThat;
66-
import static org.neo4j.driver.internal.value.NodeValueTest.emptyNodeValue;
67-
import static org.neo4j.driver.internal.value.PathValueTest.pathValue;
68-
import static org.neo4j.driver.internal.value.RelationshipValueTest.emptyRelationshipValue;
66+
import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue;
67+
import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue;
68+
import static org.neo4j.driver.internal.util.ValueFactory.pathValue;
6969
import static org.neo4j.driver.v1.Values.isoDuration;
7070
import static org.neo4j.driver.v1.Values.ofDouble;
7171
import static org.neo4j.driver.v1.Values.ofFloat;
@@ -495,7 +495,7 @@ public void shouldComplainAboutNodeType() throws Throwable
495495
{
496496
// Expect
497497
exception.expect( ClientException.class );
498-
exception.expectMessage( "Unsupported param type NODE" );
498+
exception.expectMessage( "Nodes can't be used as parameters." );
499499

500500
// When
501501
NodeValue node = emptyNodeValue();
@@ -507,7 +507,7 @@ public void shouldComplainAboutRelationshipType() throws Throwable
507507
{
508508
// Expect
509509
exception.expect( ClientException.class );
510-
exception.expectMessage( "Unsupported param type RELATIONSHIP" );
510+
exception.expectMessage( "Relationships can't be used as parameters." );
511511

512512
// When
513513
RelationshipValue rel = emptyRelationshipValue();
@@ -519,7 +519,7 @@ public void shouldComplainAboutPathType() throws Throwable
519519
{
520520
// Expect
521521
exception.expect( ClientException.class );
522-
exception.expectMessage( "Unsupported param type PATH" );
522+
exception.expectMessage( "Paths can't be used as parameters." );
523523

524524
// When
525525
PathValue path = pathValue();

‎driver/src/test/java/org/neo4j/driver/internal/util/ValueFactory.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright (c) 2002-2018 "Neo Technology,"
3+
* Network Engine for Objects in Lund AB [http://neotechnology.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal.util;
20+
21+
import java.util.HashMap;
22+
23+
import org.neo4j.driver.internal.InternalNode;
24+
import org.neo4j.driver.internal.InternalPath;
25+
import org.neo4j.driver.internal.InternalRelationship;
26+
import org.neo4j.driver.internal.value.NodeValue;
27+
import org.neo4j.driver.internal.value.PathValue;
28+
import org.neo4j.driver.internal.value.RelationshipValue;
29+
import org.neo4j.driver.v1.Value;
30+
31+
import static java.util.Collections.singletonList;
32+
import static java.util.Collections.singletonMap;
33+
import static org.neo4j.driver.v1.Values.value;
34+
35+
public class ValueFactory
36+
{
37+
public static NodeValue emptyNodeValue()
38+
{
39+
return new NodeValue( new InternalNode( 1234, singletonList( "User" ), new HashMap<String, Value>() ) );
40+
}
41+
42+
public static NodeValue filledNodeValue()
43+
{
44+
return new NodeValue( new InternalNode( 1234, singletonList( "User" ), singletonMap( "name", value( "Dodo" ) ) ) );
45+
}
46+
47+
public static RelationshipValue emptyRelationshipValue()
48+
{
49+
return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS" ) );
50+
}
51+
52+
public static RelationshipValue filledRelationshipValue()
53+
{
54+
return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS", singletonMap( "name", value( "Dodo" ) ) ) );
55+
}
56+
57+
public static PathValue pathValue()
58+
{
59+
return new PathValue( new InternalPath( new InternalNode(42L), new InternalRelationship( 43L, 42L, 44L, "T" ), new InternalNode( 44L ) ) );
60+
}
61+
}

‎driver/src/test/java/org/neo4j/driver/internal/value/NodeValueTest.java‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,15 @@
2020

2121
import org.junit.Test;
2222

23-
import java.util.HashMap;
24-
25-
import org.neo4j.driver.internal.InternalNode;
2623
import org.neo4j.driver.internal.types.InternalTypeSystem;
2724
import org.neo4j.driver.internal.types.TypeConstructor;
28-
import org.neo4j.driver.v1.Value;
2925

30-
import static java.util.Collections.singletonList;
31-
import static java.util.Collections.singletonMap;
3226
import static org.hamcrest.CoreMatchers.equalTo;
3327
import static org.junit.Assert.assertEquals;
3428
import static org.junit.Assert.assertFalse;
3529
import static org.junit.Assert.assertThat;
36-
import static org.neo4j.driver.v1.Values.value;
30+
import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue;
31+
import static org.neo4j.driver.internal.util.ValueFactory.filledNodeValue;
3732

3833
public class NodeValueTest
3934
{
@@ -69,14 +64,4 @@ public void shouldTypeAsNode()
6964
InternalValue value = emptyNodeValue();
7065
assertThat( value.typeConstructor(), equalTo( TypeConstructor.NODE ) );
7166
}
72-
73-
public static NodeValue emptyNodeValue()
74-
{
75-
return new NodeValue( new InternalNode( 1234, singletonList( "User" ), new HashMap<String, Value>() ) );
76-
}
77-
78-
private static NodeValue filledNodeValue()
79-
{
80-
return new NodeValue( new InternalNode( 1234, singletonList( "User" ), singletonMap( "name", value( "Dodo" ) ) ) );
81-
}
8267
}

‎driver/src/test/java/org/neo4j/driver/internal/value/PathValueTest.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,14 @@
2020

2121
import org.junit.Test;
2222

23-
import org.neo4j.driver.internal.InternalNode;
24-
import org.neo4j.driver.internal.InternalPath;
25-
import org.neo4j.driver.internal.InternalRelationship;
2623
import org.neo4j.driver.internal.types.InternalTypeSystem;
2724
import org.neo4j.driver.v1.Value;
2825

2926
import static org.hamcrest.CoreMatchers.equalTo;
3027
import static org.junit.Assert.assertEquals;
3128
import static org.junit.Assert.assertFalse;
3229
import static org.junit.Assert.assertThat;
30+
import static org.neo4j.driver.internal.util.ValueFactory.pathValue;
3331

3432
public class PathValueTest
3533
{
@@ -53,9 +51,4 @@ public void shouldHaveCorrectType() throws Throwable
5351

5452
assertThat(pathValue().type(), equalTo( InternalTypeSystem.TYPE_SYSTEM.PATH() ));
5553
}
56-
57-
public static PathValue pathValue()
58-
{
59-
return new PathValue( new InternalPath( new InternalNode(42L), new InternalRelationship( 43L, 42L, 44L, "T" ), new InternalNode( 44L ) ) );
60-
}
6154
}

‎driver/src/test/java/org/neo4j/driver/internal/value/RelationshipValueTest.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@
2020

2121
import org.junit.Test;
2222

23-
import org.neo4j.driver.internal.InternalRelationship;
2423
import org.neo4j.driver.internal.types.TypeConstructor;
2524
import org.neo4j.driver.v1.Value;
2625

27-
import static java.util.Collections.singletonMap;
2826
import static org.hamcrest.CoreMatchers.equalTo;
2927
import static org.junit.Assert.assertEquals;
3028
import static org.junit.Assert.assertFalse;
3129
import static org.junit.Assert.assertThat;
32-
import static org.neo4j.driver.v1.Values.value;
30+
import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue;
31+
import static org.neo4j.driver.internal.util.ValueFactory.filledRelationshipValue;
3332

3433
public class RelationshipValueTest
3534
{
@@ -60,14 +59,4 @@ public void shouldTypeAsRelationship()
6059
InternalValue value = emptyRelationshipValue();
6160
assertThat( value.typeConstructor(), equalTo( TypeConstructor.RELATIONSHIP ) );
6261
}
63-
64-
public static RelationshipValue emptyRelationshipValue()
65-
{
66-
return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS" ) );
67-
}
68-
69-
private static RelationshipValue filledRelationshipValue()
70-
{
71-
return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS", singletonMap( "name", value( "Dodo" ) ) ) );
72-
}
7362
}

0 commit comments

Comments
 (0)