Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed handling of empty byte arrays #430

Merged
merged 1 commit into from
Nov 16, 2017
Merged

Fixed handling of empty byte arrays #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions driver/src/main/java/org/neo4j/driver/internal/net/BufferingChunkedInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,11 @@ public double readDouble() throws IOException
@Override
public PackInput readBytes( byte[] into, int offset, int toRead ) throws IOException
{
ByteBuffer dst = ByteBuffer.wrap( into, offset, toRead );
read( dst );
if ( toRead != 0 )
{
ByteBuffer dst = ByteBuffer.wrap( into, offset, toRead );
read( dst );
}
return this;
}

Expand Down
10 changes: 8 additions & 2 deletions driver/src/main/java/org/neo4j/driver/internal/packstream/PackStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@
* <tr><td><code>DB</code></td><td><code>11011011</code></td><td><em>RESERVED</em></td><td></td></tr>
* <tr><td><code>DC</code></td><td><code>11011100</code></td><td>STRUCT_8</td><td>Structure (fewer than 2<sup>8</sup> fields)</td></tr>
* <tr><td><code>DD</code></td><td><code>11011101</code></td><td>STRUCT_16</td><td>Structure (fewer than 2<sup>16</sup> fields)</td></tr>
* <tr><td><code>DE</code></td><td><code>11011110</code></td><td>STRUCT_32</td><td>Structure (fewer than 2<sup>32</sup> fields)</td></tr>
* <tr><td><code>DE</code></td><td><code>11011110</code></td><td><em>RESERVED</em></td><td></td></tr>
* <tr><td><code>DF</code></td><td><code>11011111</code></td><td><em>RESERVED</em></td><td></td></tr>
* <tr><td><code>DF</code></td><td><code>11011111</code></td><td><em>RESERVED</em></td><td></td></tr>
* <tr><td><code>E0..EF</code></td><td><code>1110xxxx</code></td><td><em>RESERVED</em></td><td></td></tr>
* <tr><td><code>F0..FF</code></td><td><code>1111xxxx</code></td><td>-TINY_INT</td><td>Integer -1 to -16</td></tr>
Expand Down Expand Up @@ -115,7 +116,7 @@ public class PackStream
public static final byte RESERVED_DB = (byte) 0xDB;
public static final byte STRUCT_8 = (byte) 0xDC;
public static final byte STRUCT_16 = (byte) 0xDD;
public static final byte RESERVED_DE = (byte) 0xDE; // TODO STRUCT_32? or the class javadoc is wrong?
public static final byte RESERVED_DE = (byte) 0xDE;
public static final byte RESERVED_DF = (byte) 0xDF;
public static final byte RESERVED_E0 = (byte) 0xE0;
public static final byte RESERVED_E1 = (byte) 0xE1;
Expand Down Expand Up @@ -144,6 +145,7 @@ public class PackStream
private static final long MINUS_2_TO_THE_31 = -2147483648L;

private static final String EMPTY_STRING = "";
private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
private static final Charset UTF_8 = Charset.forName( "UTF-8" );

private PackStream() {}
Expand Down Expand Up @@ -614,6 +616,10 @@ private long unpackUINT32() throws IOException

private byte[] unpackRawBytes(int size ) throws IOException
{
if ( size == 0 )
{
return EMPTY_BYTE_ARRAY;
}
byte[] heapBuffer = new byte[size];
in.readBytes( heapBuffer, 0, heapBuffer.length );
return heapBuffer;
Expand Down
13 changes: 13 additions & 0 deletions driver/src/test/java/org/neo4j/driver/internal/net/BufferingChunkedInputTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class BufferingChunkedInputTest
Expand Down Expand Up @@ -515,6 +517,17 @@ public void shouldKeepBufferCorrectWhenError() throws Throwable
assertFalse( channel.isOpen() );
}

@Test
public void shouldNotReadFromChannelWhenEmptyByteBufferRequested() throws IOException
{
ReadableByteChannel channel = mock( ReadableByteChannel.class );
BufferingChunkedInput input = new BufferingChunkedInput( channel );

input.readBytes( new byte[0], 0, 0 );

verify( channel, never() ).read( any( ByteBuffer.class ) );
}

private ReadableByteChannel fillPacket( int size, int value )
{
int[] ints = new int[size];
Expand Down
28 changes: 15 additions & 13 deletions driver/src/test/java/org/neo4j/driver/internal/packstream/PackStreamTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,26 +326,28 @@ public void testCanPackAndUnpackPowersOfTwoMinusABitAsDoubles() throws Throwable
@Test
public void testCanPackAndUnpackByteArrays() throws Throwable
{
// Given
Machine machine = new Machine( 17000000 );

testByteArrayPackingAndUnpacking( machine, 0 );
for ( int i = 0; i < 24; i++ )
{
byte[] array = new byte[(int) Math.pow( 2, i )];
testByteArrayPackingAndUnpacking( machine, (int) Math.pow( 2, i ) );
}
}

// When
machine.reset();
machine.packer().pack( array );
machine.packer().flush();
private void testByteArrayPackingAndUnpacking( Machine machine, int length ) throws Throwable
{
byte[] array = new byte[length];

// Then
PackStream.Unpacker unpacker = newUnpacker( machine.output() );
PackType packType = unpacker.peekNextType();
machine.reset();
machine.packer().pack( array );
machine.packer().flush();

// Then
assertThat( packType, equalTo( PackType.BYTES ) );
assertArrayEquals( array, unpacker.unpackBytes() );
}
PackStream.Unpacker unpacker = newUnpacker( machine.output() );
PackType packType = unpacker.peekNextType();

assertThat( packType, equalTo( PackType.BYTES ) );
assertArrayEquals( array, unpacker.unpackBytes() );
}

@Test
Expand Down
81 changes: 51 additions & 30 deletions driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.concurrent.ThreadLocalRandom;

import org.neo4j.driver.internal.util.ServerVersion;
import org.neo4j.driver.v1.Record;
import org.neo4j.driver.v1.StatementResult;
Expand Down Expand Up @@ -150,30 +152,17 @@ public void shouldBeAbleToSetAndReturnDoubleProperty()
}
}

private boolean supportsBytes()
{
String versionString = session.run( "RETURN 1" ).consume().server().version();
return version( versionString ).greaterThanOrEqual( ServerVersion.v3_2_0 );
}

@Test
public void shouldBeAbleToSetAndReturnBytesProperty()
{
assumeTrue( supportsBytes() );

// Given
byte[] byteArray = "hello, world".getBytes();

// When
StatementResult result = session.run(
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", byteArray ) );

// Then
for ( Record record : result.list() )
testBytesProperty( new byte[0] );
for ( int i = 0; i < 16; i++ )
{
Value value = record.get( "a.value" );
assertThat( value.hasType( session.typeSystem().BYTES() ), equalTo( true ) );
assertThat( value.asByteArray(), equalTo( byteArray ) );
int length = (int) Math.pow( 2, i );
testBytesProperty( randomByteArray( length ) );
testBytesProperty( randomByteArray( length - 1 ) );
}
}

Expand Down Expand Up @@ -202,18 +191,10 @@ public void shouldThrowExceptionWhenServerDoesNotSupportBytes()
@Test
public void shouldBeAbleToSetAndReturnStringProperty()
{
// When
StatementResult result = session.run(
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", "Mjölnir" ) );

// Then
for ( Record record : result.list() )
{
Value value = record.get( "a.value" );
assertThat( value.hasType( session.typeSystem().STRING() ), equalTo( true ) );
assertThat( value.asString(), equalTo( "Mjölnir" ) );
}

testStringProperty( "" );
testStringProperty( "π≈3.14" );
testStringProperty( "Mjölnir" );
testStringProperty( "*** Hello World! ***" );
}

@Test
Expand Down Expand Up @@ -460,4 +441,44 @@ public void shouldNotBePossibleToUsePathAsParameter()
// WHEN
session.run( "RETURN {a}", parameters( "a", path ) );
}

private void testBytesProperty( byte[] array )
{
assumeTrue( supportsBytes() );

StatementResult result = session.run(
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", array ) );

for ( Record record : result.list() )
{
Value value = record.get( "a.value" );
assertThat( value.hasType( session.typeSystem().BYTES() ), equalTo( true ) );
assertThat( value.asByteArray(), equalTo( array ) );
}
}

private void testStringProperty( String string )
{
StatementResult result = session.run(
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", string ) );

for ( Record record : result.list() )
{
Value value = record.get( "a.value" );
assertThat( value.hasType( session.typeSystem().STRING() ), equalTo( true ) );
assertThat( value.asString(), equalTo( string ) );
}
}

private boolean supportsBytes()
{
return version( session.driver() ).greaterThanOrEqual( ServerVersion.v3_2_0 );
}

private static byte[] randomByteArray( int length )
{
byte[] result = new byte[length];
ThreadLocalRandom.current().nextBytes( result );
return result;
}
}