Merge pull request #357 from lutovich/1.3-routing-ctx-parsing

Prohibit empty keys in routing context

Author: zhenlineo

driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingContext.java

@@ -51,6 +51,12 @@ public Map<String,String> asMap()
         return context;
     }
 
+    @Override
+    public String toString()
+    {
+        return "RoutingContext" + context;
+    }
+
     private static Map<String,String> parseParameters( URI uri )
     {
         String query = uri.getQuery();
@@ -71,8 +77,12 @@ private static Map<String,String> parseParameters( URI uri )
                         "Invalid parameters: '" + pair + "' in URI '" + uri + "'" );
             }
 
-            String key = keyValue[0];
-            String value = keyValue[1];
+            String key = trimAndVerify( keyValue[0], "key", uri );
+            String value = trimAndVerify( keyValue[1], "value", uri );
+            if ( value.isEmpty() )
+            {
+                throw new IllegalArgumentException( "Illegal empty value in URI query '" + uri + "'" );
+            }
             String previousValue = parameters.put( key, value );
 
             if ( previousValue != null )
@@ -83,4 +93,14 @@ private static Map<String,String> parseParameters( URI uri )
         }
         return parameters;
     }
+
+    private static String trimAndVerify( String string, String name, URI uri )
+    {
+        String result = string.trim();
+        if ( result.isEmpty() )
+        {
+            throw new IllegalArgumentException( "Illegal empty " + name + " in URI query '" + uri + "'" );
+        }
+        return result;
+    }
 }

driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingContextTest.java

@@ -49,11 +49,14 @@ public void emptyContextInEmptyMap()
     @Test
     public void uriWithoutQueryIsParsedToEmptyContext()
     {
-        URI uri = URI.create( "bolt+routing://localhost:7687/" );
-        RoutingContext context = new RoutingContext( uri );
+        testEmptyRoutingContext( URI.create( "bolt+routing://localhost:7687/" ) );
+    }
 
-        assertFalse( context.isDefined() );
-        assertTrue( context.asMap().isEmpty() );
+    @Test
+    public void uriWithEmptyQueryIsParsedToEmptyContext()
+    {
+        testEmptyRoutingContext( URI.create( "bolt+routing://localhost:7687?" ) );
+        testEmptyRoutingContext( URI.create( "bolt+routing://localhost:7687/?" ) );
     }
 
     @Test
@@ -73,33 +76,25 @@ public void uriWithQueryIsParsed()
     @Test
     public void throwsForInvalidUriQuery()
     {
-        URI uri = URI.create( "bolt+routing://localhost:7687/?justKey" );
+        testIllegalUri( URI.create( "bolt+routing://localhost:7687/?justKey" ) );
+    }
 
-        try
-        {
-            new RoutingContext( uri );
-            fail( "Exception expected" );
-        }
-        catch ( Exception e )
-        {
-            assertThat( e, instanceOf( IllegalArgumentException.class ) );
-        }
+    @Test
+    public void throwsForInvalidUriQueryKey()
+    {
+        testIllegalUri( URI.create( "bolt+routing://localhost:7687/?=value1&key2=value2" ) );
     }
 
     @Test
-    public void throwsForDuplicatedUriQueryParameters()
+    public void throwsForInvalidUriQueryValue()
     {
-        URI uri = URI.create( "bolt+routing://localhost:7687/?key1=value1&key2=value2&key1=value2" );
+        testIllegalUri( URI.create( "bolt+routing://localhost:7687/key1?=value1&key2=" ) );
+    }
 
-        try
-        {
-            new RoutingContext( uri );
-            fail( "Exception expected" );
-        }
-        catch ( Exception e )
-        {
-            assertThat( e, instanceOf( IllegalArgumentException.class ) );
-        }
+    @Test
+    public void throwsForDuplicatedUriQueryParameters()
+    {
+        testIllegalUri( URI.create( "bolt+routing://localhost:7687/?key1=value1&key2=value2&key1=value2" ) );
     }
 
     @Test
@@ -122,4 +117,25 @@ public void mapRepresentationIsUnmodifiable()
 
         assertEquals( singletonMap( "key1", "value1" ), context.asMap() );
     }
+
+    private static void testIllegalUri( URI uri )
+    {
+        try
+        {
+            new RoutingContext( uri );
+            fail( "Exception expected" );
+        }
+        catch ( Exception e )
+        {
+            assertThat( e, instanceOf( IllegalArgumentException.class ) );
+        }
+    }
+
+    private static void testEmptyRoutingContext( URI uri )
+    {
+        RoutingContext context = new RoutingContext( uri );
+
+        assertFalse( context.isDefined() );
+        assertTrue( context.asMap().isEmpty() );
+    }
 }