Skip to content

Commit 3e784c6

Browse files
lutovichlutovich
committed
Fixed handling of empty byte arrays
Previously decoding of empty byte arrays caused an error because pack stream layer forced pack input to fill in an empty buffer. Later tried to fetch next chunk in such case and still fill the given buffer with nothing. This caused it to fail later on message boundary, when asserting that the whole chunk has been consumed. This commit fixes the problem by making pack input do nothing when asked to fill in an empty buffer. It also adds a branch in pack stream to always return a single empty byte array and not even attempt to read.
1 parent 8cc7b03 commit 3e784c6

File tree

5 files changed

+92
-47
lines changed

5 files changed

+92
-47
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/net/BufferingChunkedInput.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,11 @@ public double readDouble() throws IOException
142142
@Override
143143
public PackInput readBytes( byte[] into, int offset, int toRead ) throws IOException
144144
{
145-
ByteBuffer dst = ByteBuffer.wrap( into, offset, toRead );
146-
read( dst );
145+
if ( toRead != 0 )
146+
{
147+
ByteBuffer dst = ByteBuffer.wrap( into, offset, toRead );
148+
read( dst );
149+
}
147150
return this;
148151
}
149152

‎driver/src/main/java/org/neo4j/driver/internal/packstream/PackStream.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@
7171
* <tr><td><code>DB</code></td><td><code>11011011</code></td><td><em>RESERVED</em></td><td></td></tr>
7272
* <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>
7373
* <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>
74-
* <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>
74+
* <tr><td><code>DE</code></td><td><code>11011110</code></td><td><em>RESERVED</em></td><td></td></tr>
75+
* <tr><td><code>DF</code></td><td><code>11011111</code></td><td><em>RESERVED</em></td><td></td></tr>
7576
* <tr><td><code>DF</code></td><td><code>11011111</code></td><td><em>RESERVED</em></td><td></td></tr>
7677
* <tr><td><code>E0..EF</code></td><td><code>1110xxxx</code></td><td><em>RESERVED</em></td><td></td></tr>
7778
* <tr><td><code>F0..FF</code></td><td><code>1111xxxx</code></td><td>-TINY_INT</td><td>Integer -1 to -16</td></tr>
@@ -115,7 +116,7 @@ public class PackStream
115116
public static final byte RESERVED_DB = (byte) 0xDB;
116117
public static final byte STRUCT_8 = (byte) 0xDC;
117118
public static final byte STRUCT_16 = (byte) 0xDD;
118-
public static final byte RESERVED_DE = (byte) 0xDE; // TODO STRUCT_32? or the class javadoc is wrong?
119+
public static final byte RESERVED_DE = (byte) 0xDE;
119120
public static final byte RESERVED_DF = (byte) 0xDF;
120121
public static final byte RESERVED_E0 = (byte) 0xE0;
121122
public static final byte RESERVED_E1 = (byte) 0xE1;
@@ -144,6 +145,7 @@ public class PackStream
144145
private static final long MINUS_2_TO_THE_31 = -2147483648L;
145146

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

149151
private PackStream() {}
@@ -614,6 +616,10 @@ private long unpackUINT32() throws IOException
614616

615617
private byte[] unpackRawBytes(int size ) throws IOException
616618
{
619+
if ( size == 0 )
620+
{
621+
return EMPTY_BYTE_ARRAY;
622+
}
617623
byte[] heapBuffer = new byte[size];
618624
in.readBytes( heapBuffer, 0, heapBuffer.length );
619625
return heapBuffer;

‎driver/src/test/java/org/neo4j/driver/internal/net/BufferingChunkedInputTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import static org.junit.Assert.fail;
4242
import static org.mockito.Matchers.any;
4343
import static org.mockito.Mockito.mock;
44+
import static org.mockito.Mockito.never;
45+
import static org.mockito.Mockito.verify;
4446
import static org.mockito.Mockito.when;
4547

4648
public class BufferingChunkedInputTest
@@ -515,6 +517,17 @@ public void shouldKeepBufferCorrectWhenError() throws Throwable
515517
assertFalse( channel.isOpen() );
516518
}
517519

520+
@Test
521+
public void shouldNotReadFromChannelWhenEmptyByteBufferRequested() throws IOException
522+
{
523+
ReadableByteChannel channel = mock( ReadableByteChannel.class );
524+
BufferingChunkedInput input = new BufferingChunkedInput( channel );
525+
526+
input.readBytes( new byte[0], 0, 0 );
527+
528+
verify( channel, never() ).read( any( ByteBuffer.class ) );
529+
}
530+
518531
private ReadableByteChannel fillPacket( int size, int value )
519532
{
520533
int[] ints = new int[size];

‎driver/src/test/java/org/neo4j/driver/internal/packstream/PackStreamTest.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -326,26 +326,28 @@ public void testCanPackAndUnpackPowersOfTwoMinusABitAsDoubles() throws Throwable
326326
@Test
327327
public void testCanPackAndUnpackByteArrays() throws Throwable
328328
{
329-
// Given
330329
Machine machine = new Machine( 17000000 );
331330

331+
testByteArrayPackingAndUnpacking( machine, 0 );
332332
for ( int i = 0; i < 24; i++ )
333333
{
334-
byte[] array = new byte[(int) Math.pow( 2, i )];
334+
testByteArrayPackingAndUnpacking( machine, (int) Math.pow( 2, i ) );
335+
}
336+
}
335337

336-
// When
337-
machine.reset();
338-
machine.packer().pack( array );
339-
machine.packer().flush();
338+
private void testByteArrayPackingAndUnpacking( Machine machine, int length ) throws Throwable
339+
{
340+
byte[] array = new byte[length];
340341

341-
// Then
342-
PackStream.Unpacker unpacker = newUnpacker( machine.output() );
343-
PackType packType = unpacker.peekNextType();
342+
machine.reset();
343+
machine.packer().pack( array );
344+
machine.packer().flush();
344345

345-
// Then
346-
assertThat( packType, equalTo( PackType.BYTES ) );
347-
assertArrayEquals( array, unpacker.unpackBytes() );
348-
}
346+
PackStream.Unpacker unpacker = newUnpacker( machine.output() );
347+
PackType packType = unpacker.peekNextType();
348+
349+
assertThat( packType, equalTo( PackType.BYTES ) );
350+
assertArrayEquals( array, unpacker.unpackBytes() );
349351
}
350352

351353
@Test

‎driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.junit.Test;
2323
import org.junit.rules.ExpectedException;
2424

25+
import java.util.concurrent.ThreadLocalRandom;
26+
2527
import org.neo4j.driver.internal.util.ServerVersion;
2628
import org.neo4j.driver.v1.Record;
2729
import org.neo4j.driver.v1.StatementResult;
@@ -150,30 +152,17 @@ public void shouldBeAbleToSetAndReturnDoubleProperty()
150152
}
151153
}
152154

153-
private boolean supportsBytes()
154-
{
155-
String versionString = session.run( "RETURN 1" ).consume().server().version();
156-
return version( versionString ).greaterThanOrEqual( ServerVersion.v3_2_0 );
157-
}
158-
159155
@Test
160156
public void shouldBeAbleToSetAndReturnBytesProperty()
161157
{
162158
assumeTrue( supportsBytes() );
163159

164-
// Given
165-
byte[] byteArray = "hello, world".getBytes();
166-
167-
// When
168-
StatementResult result = session.run(
169-
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", byteArray ) );
170-
171-
// Then
172-
for ( Record record : result.list() )
160+
testBytesProperty( new byte[0] );
161+
for ( int i = 0; i < 16; i++ )
173162
{
174-
Value value = record.get( "a.value" );
175-
assertThat( value.hasType( session.typeSystem().BYTES() ), equalTo( true ) );
176-
assertThat( value.asByteArray(), equalTo( byteArray ) );
163+
int length = (int) Math.pow( 2, i );
164+
testBytesProperty( randomByteArray( length ) );
165+
testBytesProperty( randomByteArray( length - 1 ) );
177166
}
178167
}
179168

@@ -202,18 +191,10 @@ public void shouldThrowExceptionWhenServerDoesNotSupportBytes()
202191
@Test
203192
public void shouldBeAbleToSetAndReturnStringProperty()
204193
{
205-
// When
206-
StatementResult result = session.run(
207-
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", "Mjölnir" ) );
208-
209-
// Then
210-
for ( Record record : result.list() )
211-
{
212-
Value value = record.get( "a.value" );
213-
assertThat( value.hasType( session.typeSystem().STRING() ), equalTo( true ) );
214-
assertThat( value.asString(), equalTo( "Mjölnir" ) );
215-
}
216-
194+
testStringProperty( "" );
195+
testStringProperty( "π≈3.14" );
196+
testStringProperty( "Mjölnir" );
197+
testStringProperty( "*** Hello World! ***" );
217198
}
218199

219200
@Test
@@ -460,4 +441,44 @@ public void shouldNotBePossibleToUsePathAsParameter()
460441
// WHEN
461442
session.run( "RETURN {a}", parameters( "a", path ) );
462443
}
444+
445+
private void testBytesProperty( byte[] array )
446+
{
447+
assumeTrue( supportsBytes() );
448+
449+
StatementResult result = session.run(
450+
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", array ) );
451+
452+
for ( Record record : result.list() )
453+
{
454+
Value value = record.get( "a.value" );
455+
assertThat( value.hasType( session.typeSystem().BYTES() ), equalTo( true ) );
456+
assertThat( value.asByteArray(), equalTo( array ) );
457+
}
458+
}
459+
460+
private void testStringProperty( String string )
461+
{
462+
StatementResult result = session.run(
463+
"CREATE (a {value:{value}}) RETURN a.value", parameters( "value", string ) );
464+
465+
for ( Record record : result.list() )
466+
{
467+
Value value = record.get( "a.value" );
468+
assertThat( value.hasType( session.typeSystem().STRING() ), equalTo( true ) );
469+
assertThat( value.asString(), equalTo( string ) );
470+
}
471+
}
472+
473+
private boolean supportsBytes()
474+
{
475+
return version( session.driver() ).greaterThanOrEqual( ServerVersion.v3_2_0 );
476+
}
477+
478+
private static byte[] randomByteArray( int length )
479+
{
480+
byte[] result = new byte[length];
481+
ThreadLocalRandom.current().nextBytes( result );
482+
return result;
483+
}
463484
}

0 commit comments

Comments
 (0)