Fix string chunking bug

jakewins · jakewins · commit c36e809712c7 · 2016-01-04T17:17:52.000+01:00
diff --git a/driver/src/main/java/org/neo4j/driver/internal/connector/socket/ChunkedOutput.java b/driver/src/main/java/org/neo4j/driver/internal/connector/socket/ChunkedOutput.java
@@ -33,14 +33,13 @@ public class ChunkedOutput implements PackOutput
     public static final int CHUNK_HEADER_SIZE = 2;
 
     private final ByteBuffer buffer;
+    private final WritableByteChannel channel;
 
     /** The chunk header */
     private int currentChunkHeaderOffset;
     /** Are currently in the middle of writing a chunk? */
     private boolean chunkOpen = false;
 
-    private final WritableByteChannel channel;
-
 
     public ChunkedOutput( WritableByteChannel ch )
     {
@@ -109,17 +108,16 @@ public PackOutput writeDouble( double value ) throws IOException
     @Override
     public PackOutput writeBytes( byte[] data, int offset, int length ) throws IOException
     {
-        int index = 0;
-        while ( index < length )
+        while ( offset < length )
         {
             // Ensure there is an open chunk, and that it has at least one byte of space left
             ensure(1);
 
             // Write as much as we can into the current chunk
-            int amountToWrite = Math.min( buffer.remaining(), length - index );
+            int amountToWrite = Math.min( buffer.remaining(), length - offset );
 
             buffer.put( data, offset, amountToWrite );
-            index += amountToWrite;
+            offset += amountToWrite;
         }
         return this;
     }
diff --git a/driver/src/test/java/org/neo4j/driver/internal/connector/socket/ChunkedOutputTest.java b/driver/src/test/java/org/neo4j/driver/internal/connector/socket/ChunkedOutputTest.java
@@ -79,11 +79,20 @@ public void shouldReserveSpaceForChunkHeaderWhenWriteDataToNewChunk() throws Thr
     @Test
     public void shouldSendOutDataWhoseSizeIsGreaterThanOutputBufferCapacity() throws Throwable
     {
-        out.writeBytes( new byte[16], 0, 16 );  // 2 + 16 is greater than the default max size 16
+        // Given
+        byte[] data = new byte[32];
+        for ( int i = 0; i < data.length; i++ )
+        {
+            data[i] = (byte) (i % 128);
+        }
+
+        // When
+        out.writeBytes( data, 4, 16 );
         out.messageBoundaryHook().run();
         out.flush();
 
+        // Then
         assertThat( BytePrinter.hex( channel.getBytes() ),
-                equalTo( "00 0e 00 00 00 00 00 00    00 00 00 00 00 00 00 00    00 02 00 00 00 00 " ) );
+                equalTo( "00 0c 04 05 06 07 08 09    0a 0b 0c 0d 0e 0f 00 00    " ) );
     }
 }
diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java
@@ -261,10 +261,15 @@ public void shouldBeAbleToSetAndReturnDoubleArrayProperty()
     }
 
     @Test
-    public void shouldBeAbleToSetAndReturnSpecialStringArrayProperty()
+    public void shouldBeAbleToSetAndReturnStringArrayProperty()
     {
-        // When
-        String[] arrayValue = new String[]{"Mjölnir", "Mjölnir", "Mjölnir"};
+        testStringArrayContaining( "cat" );
+        testStringArrayContaining( "Mjölnir" );
+    }
+
+    private void testStringArrayContaining( String str )
+    {
+        String[] arrayValue = new String[]{str, str, str};
 
         ResultCursor result = session.run(
                 "CREATE (a {value:{value}}) RETURN a.value", parameters( "value", arrayValue ) );
@@ -278,32 +283,31 @@ public void shouldBeAbleToSetAndReturnSpecialStringArrayProperty()
             for ( Value item : value.asList() )
             {
                 assertThat( item.hasType( session.typeSystem().STRING() ), equalTo( true ) );
-                assertThat( item.asString(), equalTo( "Mjölnir" ) );
+                assertThat( item.asString(), equalTo( str ) );
             }
         }
     }
 
     @Test
-    public void shouldBeAbleToSetAndReturnStringArrayProperty()
+    public void shouldHandleLargeString() throws Throwable
     {
-        // When
-        String[] arrayValue = new String[]{"cat", "cat", "cat"};
-        ResultCursor result = session.run(
-                "CREATE (a {value:{value}}) RETURN a.value", parameters( "value", arrayValue ) );
-
-        // Then
-        for ( Record record : result.list() )
+        // Given
+        char[] bigStr = new char[1024 * 10];
+        for ( int i = 0; i < bigStr.length; i+=4 )
         {
-            Value value = record.value( "a.value" );
-            assertThat( value.hasType( session.typeSystem().LIST() ), equalTo( true ) );
-            assertThat( value.size(), equalTo( 3 ) );
-            for ( Value item : value.asList() )
-            {
-                assertThat( item.hasType( session.typeSystem().STRING() ), equalTo( true ) );
-                assertThat( item.asString(), equalTo( "cat" ) );
-            }
+            bigStr[i] = 'a';
+            bigStr[i+1] = 'b';
+            bigStr[i+2] = 'c';
+            bigStr[i+3] = 'd';
         }
 
+        String bigString = new String( bigStr );
+
+        // When
+        Value val = session.run( "RETURN {p} AS p", parameters( "p", bigString ) ).peek().value( "p" );
+
+        // Then
+        assertThat( val.asString(), equalTo( bigString ) );
     }
 
     @Test
