Skip to content

Commit

Permalink
Fixing a bug in packing of size of MAP, LIST, STRING, and BYTES in Pa…
Browse files Browse the repository at this point in the history 
…ckStream.

The size in the range from 2^7 to 2^8-1 was mistakenly packed in 2 bytes rather than 1 bytes.
The size in the range from 2^15 to 2^16-1 was mistakenly packed in 4 bytes rather than 2 bytes.
  • Loading branch information
@zhenlineo
Zhen Li authored and zhenlineo committed Nov 28, 2018
1 parent a2e0549 commit 30f5fca
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 85 deletions.
2 changes: 1 addition & 1 deletion community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPackV1.java
View file
Expand Up @@ -553,7 +553,7 @@ public MapValue unpackMap() throws IOException
MapValueBuilder map;
if ( size == UNKNOWN_SIZE )
{
map = new MapValueBuilder( );
map = new MapValueBuilder();
boolean more = true;
while ( more )
{
Expand Down
136 changes: 53 additions & 83 deletions community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java
View file
Expand Up @@ -153,6 +153,8 @@ public class PackStream

private static final long PLUS_2_TO_THE_31 = 2147483648L;
private static final long PLUS_2_TO_THE_15 = 32768L;
private static final long PLUS_2_TO_THE_16 = 65536L;
private static final long PLUS_2_TO_THE_8 = 256L;
private static final long PLUS_2_TO_THE_7 = 128L;
private static final long MINUS_2_TO_THE_4 = -16L;
private static final long MINUS_2_TO_THE_7 = -128L;
Expand Down Expand Up @@ -351,58 +353,17 @@ public void packUTF8( byte[] bytes, int offset, int length ) throws IOException

protected void packBytesHeader( int size ) throws IOException
{
if ( size <= Byte.MAX_VALUE )
{
out.writeShort( (short) (BYTES_8 << 8 | size) );
}
else if ( size <= Short.MAX_VALUE )
{
out.writeByte( BYTES_16 ).writeShort( (short) size );
}
else
{
out.writeByte( BYTES_32 ).writeInt( size );
}
packHeader( size, BYTES_8, BYTES_16, BYTES_32 );
}

private void packStringHeader( int size ) throws IOException
void packStringHeader( int size ) throws IOException
{
if ( size < 0x10 )
{
out.writeByte( (byte) (TINY_STRING | size) );
}
else if ( size <= Byte.MAX_VALUE )
{
out.writeShort( (short) (STRING_8 << 8 | size) );
}
else if ( size <= Short.MAX_VALUE )
{
out.writeByte( STRING_16 ).writeShort( (short) size );
}
else
{
out.writeByte( STRING_32 ).writeInt( size );
}
packHeader( size, TINY_STRING, STRING_8, STRING_16, STRING_32 );
}

public void packListHeader( int size ) throws IOException
{
if ( size < 0x10 )
{
out.writeByte( (byte) (TINY_LIST | size) );
}
else if ( size <= Byte.MAX_VALUE )
{
out.writeShort( (short) (LIST_8 << 8 | size) );
}
else if ( size <= Short.MAX_VALUE )
{
out.writeByte( LIST_16 ).writeShort( (short) size );
}
else
{
out.writeByte( LIST_32 ).writeInt( size );
}
packHeader( size, TINY_LIST, LIST_8, LIST_16, LIST_32 );
}

public void packListStreamHeader() throws IOException
Expand All @@ -412,22 +373,7 @@ public void packListStreamHeader() throws IOException

public void packMapHeader( int size ) throws IOException
{
if ( size < 0x10 )
{
out.writeByte( (byte) (TINY_MAP | size) );
}
else if ( size <= Byte.MAX_VALUE )
{
out.writeShort( (short) (MAP_8 << 8 | size) );
}
else if ( size <= Short.MAX_VALUE )
{
out.writeByte( MAP_16 ).writeShort( (short) size );
}
else
{
out.writeByte( MAP_32 ).writeInt( size );
}
packHeader( size, TINY_MAP, MAP_8, MAP_16, MAP_32 );
}

public void packMapStreamHeader() throws IOException
Expand All @@ -441,11 +387,11 @@ public void packStructHeader( int size, byte signature ) throws IOException
{
out.writeShort( (short) ((byte) (TINY_STRUCT | size) << 8 | (signature & 0xFF)) );
}
else if ( size <= Byte.MAX_VALUE )
else if ( size < PLUS_2_TO_THE_8 )
{
out.writeByte( STRUCT_8 ).writeByte( (byte) size ).writeByte( signature );
}
else if ( size <= Short.MAX_VALUE )
else if ( size < PLUS_2_TO_THE_16 )
{
out.writeByte( STRUCT_16 ).writeShort( (short) size ).writeByte( signature );
}
Expand All @@ -460,6 +406,33 @@ public void packEndOfStream() throws IOException
out.writeByte( END_OF_STREAM );
}

private void packHeader( int size, byte marker8, byte marker16, byte marker32 ) throws IOException
{
if ( size < PLUS_2_TO_THE_8 )
{
out.writeShort( (short) (marker8 << 8 | size) );
}
else if ( size < PLUS_2_TO_THE_16 )
{
out.writeByte( marker16 ).writeShort( (short) size );
}
else
{
out.writeByte( marker32 ).writeInt( size );
}
}

private void packHeader( int size, byte marker4, byte marker8, byte marker16, byte marker32 ) throws IOException
{
if ( size < 0x10 )
{
out.writeByte( (byte) (marker4 | size) );
}
else
{
packHeader( size, marker8, marker16, marker32 );
}
}
}

public static class Unpacker
Expand Down Expand Up @@ -519,7 +492,7 @@ public long unpackListHeader() throws IOException
case LIST_16:
return unpackUINT16();
case LIST_32:
return unpackUINT32();
return unpackUINT32( PackType.LIST );
case LIST_STREAM:
return UNKNOWN_SIZE;
default:
Expand All @@ -544,7 +517,7 @@ public long unpackMapHeader() throws IOException
case MAP_16:
return unpackUINT16();
case MAP_32:
return unpackUINT32();
return unpackUINT32( PackType.MAP );
case MAP_STREAM:
return UNKNOWN_SIZE;
default:
Expand Down Expand Up @@ -631,15 +604,7 @@ public int unpackBytesHeader() throws IOException
break;
case BYTES_32:
{
long longSize = unpackUINT32();
if ( longSize <= Integer.MAX_VALUE )
{
size = (int) longSize;
}
else
{
throw new Overflow( "BYTES_32 too long for Java" );
}
size = unpackUINT32( PackType.BYTES );
break;
}
default:
Expand Down Expand Up @@ -672,15 +637,7 @@ public int unpackStringHeader() throws IOException
break;
case STRING_32:
{
long longSize = unpackUINT32();
if ( longSize <= Integer.MAX_VALUE )
{
size = (int) longSize;
}
else
{
throw new Overflow( "STRING_32 too long for Java" );
}
size = unpackUINT32( PackType.STRING );
break;
}
default:
Expand Down Expand Up @@ -732,6 +689,19 @@ private long unpackUINT32() throws IOException
return in.readInt() & 0xFFFFFFFFL;
}

private int unpackUINT32( PackType type ) throws IOException
{
long longSize = unpackUINT32();
if ( longSize <= Integer.MAX_VALUE )
{
return (int) longSize;
}
else
{
throw new Overflow( String.format( "%s_32 too long for Java", type ) );
}
}

public void unpackEndOfStream() throws IOException
{
final byte markerByte = in.readByte();
Expand Down

0 comments on commit 30f5fca

Please sign in to comment.