Skip to content

Commit

Permalink
GenSafePointer now assert pointer being inside boundary box
Browse files Browse the repository at this point in the history
  • Loading branch information
@burqen @tinwelint
burqen authored and tinwelint committed Dec 6, 2016
1 parent 415b54a commit f0e79cd
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 26 deletions.
15 changes: 14 additions & 1 deletion community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointer.java
View file
Expand Up @@ -47,6 +47,8 @@ class GenSafePointer
// unsigned int
static final long MAX_GENERATION = 0xFFFFFFFFL;
static final long GENERATION_MASK = 0xFFFFFFFFL;
static final long MIN_POINTER = IdSpace.MIN_TREE_NODE_ID;
static final long MAX_POINTER = 0xFFFF_FFFFFFFFL;
static final int UNSIGNED_SHORT_MASK = 0xFFFF;

static final int CHECKSUM_SIZE = 2;
Expand All @@ -65,6 +67,7 @@ class GenSafePointer
public static void write( PageCursor cursor, long generation, long pointer )
{
assertGeneration( generation );
assertPointer( pointer );
cursor.putInt( (int) generation );
put6BLong( cursor, pointer );
cursor.putShort( checksumOf( generation, pointer ) );
Expand All @@ -79,6 +82,15 @@ static void assertGeneration( long generation )
}
}

private static void assertPointer( long pointer )
{
if ( (pointer > MAX_POINTER || pointer < MIN_POINTER) && pointer != TreeNode.NO_NODE_FLAG )
{
throw new IllegalArgumentException( "Can not write pointer " + pointer +
" because outside boundary for valid pointer" );
}
}

public static long readGeneration( PageCursor cursor )
{
return cursor.getInt() & GENERATION_MASK;
Expand Down Expand Up @@ -108,7 +120,8 @@ private static long get6BLong( PageCursor cursor )
return lsb | (msb << Integer.SIZE);
}

private static void put6BLong( PageCursor cursor, long value )
// package visible for test purposes
static void put6BLong( PageCursor cursor, long value )
{
int lsb = (int) value;
short msb = (short) (value >>> Integer.SIZE);
Expand Down
16 changes: 16 additions & 0 deletions community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointerPair.java
View file
Expand Up @@ -438,6 +438,22 @@ static String failureDescription( long result )
return builder.toString();
}

/**
* Asserts that a result is {@link #isSuccess(long) successful}, otherwise throws {@link IllegalStateException}.
*
* @param result result returned from {@link #read(PageCursor, long, long)} or
* {@link #write(PageCursor, long, long, long)}
* @return {@code true} if {@link #isSuccess(long) successful}, for interoperability with {@code assert}.
*/
static boolean assertSuccess( long result )
{
if ( !isSuccess( result ) )
{
throw new IllegalStateException( failureDescription( result ) );
}
return true;
}

private static String generationComparisonFromResult( long result )
{
long bits = result & GEN_COMPARISON_MASK;
Expand Down
9 changes: 6 additions & 3 deletions community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java
View file
Expand Up @@ -179,19 +179,22 @@ void setKeyCount( PageCursor cursor, int count )
void setRightSibling( PageCursor cursor, long rightSiblingId, long stableGeneration, long unstableGeneration )
{
cursor.setOffset( BYTE_POS_RIGHTSIBLING );
GenSafePointerPair.write( cursor, rightSiblingId, stableGeneration, unstableGeneration );
long result = GenSafePointerPair.write( cursor, rightSiblingId, stableGeneration, unstableGeneration );
GenSafePointerPair.assertSuccess( result );
}

void setLeftSibling( PageCursor cursor, long leftSiblingId, long stableGeneration, long unstableGeneration )
{
cursor.setOffset( BYTE_POS_LEFTSIBLING );
GenSafePointerPair.write( cursor, leftSiblingId, stableGeneration, unstableGeneration );
long result = GenSafePointerPair.write( cursor, leftSiblingId, stableGeneration, unstableGeneration );
GenSafePointerPair.assertSuccess( result );
}

void setNewGen( PageCursor cursor, long newGenId, long stableGeneration, long unstableGeneration )
{
cursor.setOffset( BYTE_POS_NEWGEN );
GenSafePointerPair.write( cursor, newGenId, stableGeneration, unstableGeneration );
long result = GenSafePointerPair.write( cursor, newGenId, stableGeneration, unstableGeneration );
GenSafePointerPair.assertSuccess( result );
}

// BODY METHODS
Expand Down
99 changes: 90 additions & 9 deletions community/index/src/test/java/org/neo4j/index/gbptree/GenSafePointerTest.java
View file
Expand Up @@ -28,6 +28,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class GenSafePointerTest
{
Expand Down Expand Up @@ -73,8 +74,6 @@ public void shouldDetectInvalidChecksumOnReadDueToChangedGeneration() throws Exc
// GIVEN
int offset = 0;
GSP initial = gsp( 123, 456 );
initial.generation = 123;
initial.pointer = 456;
write( cursor, offset, initial );

// WHEN
Expand All @@ -91,8 +90,6 @@ public void shouldDetectInvalidChecksumOnReadDueToChangedChecksum() throws Excep
// GIVEN
int offset = 0;
GSP initial = gsp( 123, 456 );
initial.generation = 123;
initial.pointer = 456;
write( cursor, offset, initial );

// WHEN
Expand All @@ -108,7 +105,7 @@ public void shouldDetectInvalidChecksumOnReadDueToChangedChecksum() throws Excep
public void shouldWriteAndReadGspCloseToGenerationMax() throws Exception
{
// GIVEN
long generation = 0xFFFFFFFEL;
long generation = GenSafePointer.MAX_GENERATION;
GSP expected = gsp( generation, 12345 );
write( cursor, 0, expected );

Expand All @@ -126,7 +123,7 @@ public void shouldWriteAndReadGspCloseToGenerationMax() throws Exception
public void shouldWriteAndReadGspCloseToPointerMax() throws Exception
{
// GIVEN
long pointer = 0xFFFF_FFFFFFFEL;
long pointer = GenSafePointer.MAX_POINTER;
GSP expected = gsp( 12345, pointer );
write( cursor, 0, expected );

Expand All @@ -144,8 +141,8 @@ public void shouldWriteAndReadGspCloseToPointerMax() throws Exception
public void shouldWriteAndReadGspCloseToGenerationAndPointerMax() throws Exception
{
// GIVEN
long generation = 0xFFFFFFFEL;
long pointer = 0xFFFF_FFFFFFFEL;
long generation = GenSafePointer.MAX_GENERATION;
long pointer = GenSafePointer.MAX_POINTER;
GSP expected = gsp( generation, pointer );
write( cursor, 0, expected );

Expand All @@ -160,6 +157,90 @@ public void shouldWriteAndReadGspCloseToGenerationAndPointerMax() throws Excepti
assertEquals( pointer, read.pointer );
}

@Test
public void shouldThrowIfPointerToLarge() throws Exception
{
// GIVEN
long generation = GenSafePointer.MIN_GENERATION;
long pointer = GenSafePointer.MAX_POINTER + 1;
GSP broken = gsp( generation, pointer );

// WHEN
try
{
write( cursor, 0, broken );
fail( "Expected to throw" );
}
catch ( IllegalArgumentException e )
{
// THEN
// good
}
}

@Test
public void shouldThrowIfPointerToSmall() throws Exception
{
// GIVEN
long generation = GenSafePointer.MIN_GENERATION;
long pointer = GenSafePointer.MIN_POINTER - 1;
GSP broken = gsp( generation, pointer );

// WHEN
try
{
write( cursor, 0, broken );
fail( "Expected to throw" );
}
catch ( IllegalArgumentException e )
{
// THEN
// good
}
}

@Test
public void shouldThrowIfGenerationToLarge() throws Exception
{
// GIVEN
long generation = GenSafePointer.MAX_GENERATION + 1;
long pointer = GenSafePointer.MIN_POINTER;
GSP broken = gsp( generation, pointer );

// WHEN
try
{
write( cursor, 0, broken );
fail( "Expected to throw" );
}
catch ( IllegalArgumentException e )
{
// THEN
// good
}
}

@Test
public void shouldThrowIfGenerationToSmall() throws Exception
{
// GIVEN
long generation = GenSafePointer.MIN_GENERATION - 1;
long pointer = GenSafePointer.MIN_POINTER;
GSP broken = gsp( generation, pointer );

// WHEN
try
{
write( cursor, 0, broken );
fail( "Expected to throw" );
}
catch ( IllegalArgumentException e )
{
// THEN
// good
}
}

@Test
public void shouldHaveLowAccidentalChecksumCollision() throws Exception
{
Expand All @@ -173,7 +254,7 @@ public void shouldHaveLowAccidentalChecksumCollision() throws Exception
for ( int i = 0; i < count; i++ )
{
gsp.generation = random.nextLong( GenSafePointer.MAX_GENERATION );
gsp.pointer = random.nextLong( 0xFFFF_FFFFFFFFL );
gsp.pointer = random.nextLong( GenSafePointer.MAX_POINTER );
short checksum = checksumOf( gsp );
if ( i == 0 )
{
Expand Down
16 changes: 11 additions & 5 deletions community/index/src/test/java/org/neo4j/index/gbptree/PointerCheckingTest.java
View file
Expand Up @@ -31,9 +31,9 @@
public class PointerCheckingTest
{
private final PageCursor cursor = ByteArrayPageCursor.wrap( GenSafePointerPair.SIZE );
private final int firstGeneration = 1;
private final int secondGeneration = 2;
private final int thirdGeneration = 3;
private final long firstGeneration = 1;
private final long secondGeneration = 2;
private final long thirdGeneration = 3;

@Test
public void checkChildShouldThrowOnNoNode() throws Exception
Expand Down Expand Up @@ -166,11 +166,17 @@ public void checkSiblingShouldThrowOnReadFailure() throws Exception
public void checkSiblingShouldThrowOnReadIllegalPointer() throws Exception
{
// GIVEN
GenSafePointer.write( cursor, secondGeneration, IdSpace.STATE_PAGE_A );
long generation = IdSpace.STATE_PAGE_A;
long pointer = this.secondGeneration;

// Can not use GenSafePointer.write because it will fail on pointer assertion.
cursor.putInt( (int) pointer );
GenSafePointer.put6BLong( cursor, generation );
cursor.putShort( GenSafePointer.checksumOf( generation, pointer ) );
cursor.rewind();

// WHEN
long result = read( cursor, firstGeneration, secondGeneration );
long result = read( cursor, firstGeneration, pointer );

// WHEN
try
Expand Down
21 changes: 13 additions & 8 deletions community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java
View file
Expand Up @@ -305,11 +305,12 @@ public void shouldSetAndGetChild() throws Exception
public void shouldOverwriteChild() throws Exception
{
// GIVEN
long child = GenSafePointer.MIN_POINTER;
node.initializeInternal( cursor, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, 1, 0, 0, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, child, 0, 0, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );

// WHEN
long overwrittenChild = 2;
long overwrittenChild = child + 1;
node.setChildAt( cursor, overwrittenChild, 0, STABLE_GENERATION, UNSTABLE_GENERATION );

// THEN
Expand Down Expand Up @@ -405,18 +406,22 @@ public void shouldReadAndInsertValues() throws Exception
public void shouldReadAndInsertChildren() throws Exception
{
// GIVEN
long firstChild = GenSafePointer.MIN_POINTER;
long secondChild = firstChild + 1;
long thirdChild = secondChild + 1;
node.initializeInternal( cursor, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, 1, 0, 0, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, 3, 1, 1, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, firstChild, 0, 0, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );
node.insertChildAt( cursor, thirdChild, 1, 1, tmp, STABLE_GENERATION, UNSTABLE_GENERATION );

// WHEN
node.readChildrenWithInsertRecordInPosition( cursor, c -> node.writeChild( c, 2, STABLE_GENERATION, UNSTABLE_GENERATION ), 1, 3, tmp );
node.readChildrenWithInsertRecordInPosition( cursor,
c -> node.writeChild( c, secondChild, STABLE_GENERATION, UNSTABLE_GENERATION ), 1, 3, tmp );
node.writeChildren( cursor, tmp, 0, 0, 3 );

// THEN
assertEquals( 1, node.childAt( cursor, 0, STABLE_GENERATION, UNSTABLE_GENERATION ) );
assertEquals( 2, node.childAt( cursor, 1, STABLE_GENERATION, UNSTABLE_GENERATION ) );
assertEquals( 3, node.childAt( cursor, 2, STABLE_GENERATION, UNSTABLE_GENERATION ) );
assertEquals( firstChild, node.childAt( cursor, 0, STABLE_GENERATION, UNSTABLE_GENERATION ) );
assertEquals( secondChild, node.childAt( cursor, 1, STABLE_GENERATION, UNSTABLE_GENERATION ) );
assertEquals( thirdChild, node.childAt( cursor, 2, STABLE_GENERATION, UNSTABLE_GENERATION ) );
}

@Test
Expand Down

0 comments on commit f0e79cd

Please sign in to comment.