diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointer.java b/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointer.java index ee53604c051e6..c3261fab2c160 100644 --- a/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointer.java +++ b/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointer.java @@ -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; @@ -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 ) ); @@ -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; @@ -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); diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointerPair.java b/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointerPair.java index b9150cdb061a6..45d23ae4ff7d6 100644 --- a/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointerPair.java +++ b/community/index/src/main/java/org/neo4j/index/gbptree/GenSafePointerPair.java @@ -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; diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java b/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java index e6ec1b489aa54..aeec5f1cd3758 100644 --- a/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java +++ b/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java @@ -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 diff --git a/community/index/src/test/java/org/neo4j/index/gbptree/GenSafePointerTest.java b/community/index/src/test/java/org/neo4j/index/gbptree/GenSafePointerTest.java index 4c4a4045cde3f..d91da0dd7975e 100644 --- a/community/index/src/test/java/org/neo4j/index/gbptree/GenSafePointerTest.java +++ b/community/index/src/test/java/org/neo4j/index/gbptree/GenSafePointerTest.java @@ -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 { @@ -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 @@ -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 @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 { @@ -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 ) { diff --git a/community/index/src/test/java/org/neo4j/index/gbptree/PointerCheckingTest.java b/community/index/src/test/java/org/neo4j/index/gbptree/PointerCheckingTest.java index f0238f881f27f..a637822399869 100644 --- a/community/index/src/test/java/org/neo4j/index/gbptree/PointerCheckingTest.java +++ b/community/index/src/test/java/org/neo4j/index/gbptree/PointerCheckingTest.java @@ -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 @@ -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 diff --git a/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java b/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java index ac33eb5d730ee..14cc56588efc7 100644 --- a/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java +++ b/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java @@ -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 @@ -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