Skip to content

Commit 6984fa0

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 6984fa0

File tree

15 files changed

+288
-188
lines changed

15 files changed

+288
-188
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/DirectDriverTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void shouldRejectInvalidAddress()
105105
}
106106
catch ( IllegalArgumentException e )
107107
{
108-
assertThat( e.getMessage(), equalTo( "Invalid address format `*`" ) );
108+
assertThat( e.getMessage(), equalTo( "Invalid obj format `*`" ) );
109109
}
110110
}
111111

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,7 @@ public void shouldAcceptRoutingTableWithoutWritersAndThenRediscover() throws Exc
929929
try ( Driver driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:9010", config );
930930
Session session = driver.session() )
931931
{
932-
// start another router which knows about writes, use same address as the initial router
932+
// start another router which knows about writes, use same obj as the initial router
933933
router2 = StubServer.start( "acquire_endpoints.script", 9010 );
934934

935935
assertEquals( asList( "Bob", "Alice", "Tina" ), readStrings( "MATCH (n) RETURN n.name", session ) );

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

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@
5454
import org.neo4j.driver.v1.Values;
5555
import org.neo4j.driver.v1.exceptions.ClientException;
5656
import org.neo4j.driver.v1.types.IsoDuration;
57+
import org.neo4j.driver.v1.types.Node;
58+
import org.neo4j.driver.v1.types.Path;
5759
import org.neo4j.driver.v1.types.Point;
60+
import org.neo4j.driver.v1.types.Relationship;
5861

5962
import static java.util.Arrays.asList;
6063
import static org.hamcrest.CoreMatchers.equalTo;
@@ -63,9 +66,9 @@
6366
import static org.junit.Assert.assertEquals;
6467
import static org.junit.Assert.assertNotEquals;
6568
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;
69+
import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue;
70+
import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue;
71+
import static org.neo4j.driver.internal.util.ValueFactory.pathValue;
6972
import static org.neo4j.driver.v1.Values.isoDuration;
7073
import static org.neo4j.driver.v1.Values.ofDouble;
7174
import static org.neo4j.driver.v1.Values.ofFloat;
@@ -491,38 +494,74 @@ public void shouldCreateValueFromPoint3D()
491494
}
492495

493496
@Test
494-
public void shouldComplainAboutNodeType() throws Throwable
497+
public void shouldComplainAboutNodeValueType() throws Throwable
495498
{
496499
// Expect
497500
exception.expect( ClientException.class );
498-
exception.expectMessage( "Unsupported param type NODE" );
501+
exception.expectMessage( "Nodes can't be used as parameters." );
499502

500503
// When
501504
NodeValue node = emptyNodeValue();
502505
value( node );
503506
}
504507

505508
@Test
506-
public void shouldComplainAboutRelationshipType() throws Throwable
509+
public void shouldComplainAboutNodeType() throws Throwable
507510
{
508511
// Expect
509512
exception.expect( ClientException.class );
510-
exception.expectMessage( "Unsupported param type RELATIONSHIP" );
513+
exception.expectMessage( "Nodes can't be used as parameters." );
514+
515+
// When
516+
Node node = emptyNodeValue().asNode();
517+
value( node );
518+
}
519+
520+
@Test
521+
public void shouldComplainAboutRelationshipValueType() throws Throwable
522+
{
523+
// Expect
524+
exception.expect( ClientException.class );
525+
exception.expectMessage( "Relationships can't be used as parameters." );
511526

512527
// When
513528
RelationshipValue rel = emptyRelationshipValue();
514529
value( rel );
515530
}
516531

517532
@Test
518-
public void shouldComplainAboutPathType() throws Throwable
533+
public void shouldComplainAboutRelationshipType() throws Throwable
519534
{
520535
// Expect
521536
exception.expect( ClientException.class );
522-
exception.expectMessage( "Unsupported param type PATH" );
537+
exception.expectMessage( "Relationships can't be used as parameters." );
538+
539+
// When
540+
Relationship rel = emptyRelationshipValue().asRelationship();
541+
value( rel );
542+
}
543+
544+
@Test
545+
public void shouldComplainAboutPathValueType() throws Throwable
546+
{
547+
// Expect
548+
exception.expect( ClientException.class );
549+
exception.expectMessage( "Paths can't be used as parameters." );
523550

524551
// When
525552
PathValue path = pathValue();
526553
value( path );
527554
}
555+
556+
@Test
557+
public void shouldComplainAboutPathType() throws Throwable
558+
{
559+
// Expect
560+
exception.expect( ClientException.class );
561+
exception.expectMessage( "Paths can't be used as parameters." );
562+
563+
// When
564+
Path path = pathValue().asPath();
565+
value( path );
566+
}
528567
}

‎driver/src/test/java/org/neo4j/driver/internal/async/ChannelConnectorImplIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public void shouldEnforceConnectTimeout() throws Exception
158158
{
159159
ChannelConnector connector = newConnector( neo4j.authToken(), 1000 );
160160

161-
// try connect to a non-routable ip address 10.0.0.0, it will never respond
161+
// try connect to a non-routable ip obj 10.0.0.0, it will never respond
162162
ChannelFuture channelFuture = connector.connect( new BoltServerAddress( "10.0.0.0" ), bootstrap );
163163

164164
try

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ protected boolean matchesSafely( Driver driver )
141141
@Override
142142
public void describeTo( Description description )
143143
{
144-
description.appendText( "direct driver with address bolt://" ).appendValue( address );
144+
description.appendText( "direct driver with obj bolt://" ).appendValue( address );
145145
}
146146
};
147147
}

‎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+
}

0 commit comments

Comments
 (0)