diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java
index 86a093f8cc6e0..b004b5ec58095 100644
--- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java
+++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java
@@ -204,6 +204,12 @@ protected void initialiseNewStoreFile( PagedFile file ) throws IOException
createHeaderRecord( pageCursor );
}
while ( pageCursor.shouldRetry() );
+ if ( pageCursor.checkAndClearBoundsFlag() )
+ {
+ throw new UnderlyingStorageException(
+ "Out of page bounds when writing header; page size too small: " +
+ pageCache.pageSize() + " bytes.");
+ }
}
}
}
@@ -216,6 +222,27 @@ protected void initialiseNewStoreFile( PagedFile file ) throws IOException
idGeneratorFactory.create( idFileName, getNumberOfReservedLowIds(), true );
}
+ protected void checkForOutOfBounds( PageCursor pageCursor, long recordId )
+ {
+ if ( pageCursor.checkAndClearBoundsFlag() )
+ {
+ throwOutOfBoundsException( recordId );
+ }
+ }
+
+ private void throwOutOfBoundsException( long recordId )
+ {
+ RECORD record = newRecord();
+ record.setId( recordId );
+ long pageId = pageIdForRecord( recordId );
+ int offset = offsetForId( recordId );
+ throw new UnderlyingStorageException(
+ "Access to record " + record + " went out of bounds of the page. The record size is " +
+ recordSize + " bytes, and the access was at offset " + offset + " bytes into page " +
+ pageId + ", and the pages have a capacity of " + storeFile.pageSize() + " bytes. " +
+ "The mapped store file in question is " + storageFileName.getAbsolutePath() );
+ }
+
protected void createHeaderRecord( PageCursor cursor ) throws IOException
{
int offset = cursor.getOffset();
@@ -263,6 +290,12 @@ private void extractHeaderRecord() throws IOException
readHeaderAndInitializeRecordFormat( pageCursor );
}
while ( pageCursor.shouldRetry() );
+ if ( pageCursor.checkAndClearBoundsFlag() )
+ {
+ throw new UnderlyingStorageException(
+ "Out of page bounds when reading header; page size too small: " +
+ pageCache.pageSize() + " bytes.");
+ }
}
}
}
@@ -293,8 +326,8 @@ public int getRecordsPerPage()
public byte[] getRawRecordData( long id ) throws IOException
{
byte[] data = new byte[recordSize];
- long pageId = RecordPageLocationCalculator.pageIdForRecord( id, storeFile.pageSize(), recordSize );
- int offset = RecordPageLocationCalculator.offsetForId( id, storeFile.pageSize(), recordSize );
+ long pageId = pageIdForRecord( id );
+ int offset = offsetForId( id );
try ( PageCursor pageCursor = storeFile.io( pageId, PagedFile.PF_SHARED_READ_LOCK ) )
{
if ( pageCursor.next() )
@@ -305,6 +338,7 @@ public byte[] getRawRecordData( long id ) throws IOException
pageCursor.getBytes( data );
}
while ( pageCursor.shouldRetry() );
+ checkForOutOfBounds( pageCursor, id );
}
}
return data;
@@ -365,6 +399,7 @@ public boolean isInUse( long id )
recordIsInUse = isInUse( cursor );
}
while ( cursor.shouldRetry() );
+ checkForOutOfBounds( cursor, id );
}
return recordIsInUse;
}
@@ -396,7 +431,7 @@ final void rebuildIdGenerator()
openIdGenerator();
long defraggedCount = 0;
- boolean fastRebuild = doFastIdGeneratorRebuild();
+ boolean fastRebuild = isOnlyFastIdGeneratorRebuildEnabled( configuration );
try
{
@@ -426,6 +461,11 @@ final void rebuildIdGenerator()
}
}
+ protected boolean isOnlyFastIdGeneratorRebuildEnabled( Config config )
+ {
+ return config.get( Configuration.rebuild_idgenerators_fast );
+ }
+
private long rebuildIdGeneratorSlow( PageCursor cursor, int recordsPerPage, int blockSize,
long foundHighId )
throws IOException
@@ -469,6 +509,7 @@ else if ( isRecordReserved( cursor ) )
}
}
while ( cursor.shouldRetry() );
+ checkIdScanCursorBounds( cursor );
for ( int i = 0; i < defragged; i++ )
{
@@ -480,9 +521,14 @@ else if ( isRecordReserved( cursor ) )
return defragCount;
}
- protected boolean doFastIdGeneratorRebuild()
+ private void checkIdScanCursorBounds( PageCursor cursor )
{
- return configuration.get( Configuration.rebuild_idgenerators_fast );
+ if ( cursor.checkAndClearBoundsFlag() )
+ {
+ throw new UnderlyingStorageException(
+ "Out of bounds access on page " + cursor.getCurrentPageId() + " detected while scanning the " +
+ storageFileName + " file for deleted records" );
+ }
}
/**
@@ -653,11 +699,14 @@ protected long scanForHighId()
long nextPageId = storeFile.getLastPageId();
int recordsPerPage = getRecordsPerPage();
int recordSize = getRecordSize();
+ long highestId = getNumberOfReservedLowIds();
while ( nextPageId >= 0 && cursor.next( nextPageId ) )
{
nextPageId--;
+ boolean found;
do
{
+ found = false;
int currentRecord = recordsPerPage;
while ( currentRecord-- > 0 )
{
@@ -671,12 +720,19 @@ protected long scanForHighId()
if ( !justLegacyStoreTrailer )
{
// We've found the highest id in use
- return recordId + 1 /*+1 since we return the high id*/;
+ found = true;
+ highestId = recordId + 1; /*+1 since we return the high id*/;
+ break;
}
}
}
}
while ( cursor.shouldRetry() );
+ checkIdScanCursorBounds( cursor );
+ if ( found )
+ {
+ return highestId;
+ }
}
return getNumberOfReservedLowIds();
@@ -986,6 +1042,7 @@ public RECORD getRecord( long id, RECORD record, RecordLoad mode )
recordFormat.read( record, cursor, mode, recordSize, storeFile );
}
while ( cursor.shouldRetry() );
+ checkForOutOfBounds( cursor, id );
verifyAfterReading( record, mode );
}
else
@@ -1018,6 +1075,7 @@ public void updateRecord( RECORD record )
recordFormat.write( record, cursor, recordSize, storeFile );
}
while ( cursor.shouldRetry() );
+ checkForOutOfBounds( cursor, id ); // We don't free ids if something weird goes wrong
if ( !record.inUse() )
{
freeId( id );
@@ -1112,6 +1170,7 @@ public boolean next()
recordFormat.read( record, pageCursor, mode, recordSize, storeFile );
}
while ( pageCursor.shouldRetry() );
+ checkForOutOfBounds( pageCursor, currentId );
verifyAfterReading( record, mode );
}
else
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TokenStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TokenStore.java
index 65e6fdda3ed82..c0491bb43ca68 100644
--- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TokenStore.java
+++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TokenStore.java
@@ -76,7 +76,7 @@ public DynamicStringStore getNameStore()
}
@Override
- protected boolean doFastIdGeneratorRebuild()
+ protected boolean isOnlyFastIdGeneratorRebuildEnabled( Config config )
{
return false;
}
diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreBehaviourTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreBehaviourTest.java
new file mode 100644
index 0000000000000..b37faee16ff8b
--- /dev/null
+++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreBehaviourTest.java
@@ -0,0 +1,321 @@
+/*
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+package org.neo4j.kernel.impl.store;
+
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import org.neo4j.function.ThrowingAction;
+import org.neo4j.graphdb.factory.GraphDatabaseSettings;
+import org.neo4j.io.pagecache.PageCache;
+import org.neo4j.io.pagecache.PageCursor;
+import org.neo4j.io.pagecache.PagedFile;
+import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.impl.store.format.BaseRecordFormat;
+import org.neo4j.kernel.impl.store.id.DefaultIdGeneratorFactory;
+import org.neo4j.kernel.impl.store.id.IdType;
+import org.neo4j.kernel.impl.store.record.AbstractBaseRecord;
+import org.neo4j.kernel.impl.store.record.RecordLoad;
+import org.neo4j.logging.NullLogProvider;
+import org.neo4j.test.EphemeralFileSystemRule;
+import org.neo4j.test.PageCacheRule;
+
+import static org.junit.Assert.fail;
+import static org.neo4j.helpers.collection.MapUtil.stringMap;
+import static org.neo4j.kernel.impl.store.record.RecordLoad.NORMAL;
+
+/**
+ * Test for {@link CommonAbstractStore}, but without using mocks.
+ * @see CommonAbstractStoreTest for testing with mocks.
+ */
+public class CommonAbstractStoreBehaviourTest
+{
+ /**
+ * Note that tests MUST use the non-modifying {@link Config#with(Map, Class[])} method, to make alternate copies
+ * of this settings class.
+ */
+ private static final Config CONFIG = Config.empty().augment( stringMap(
+ GraphDatabaseSettings.pagecache_memory.name(), "8M" ) );
+
+ private final EphemeralFileSystemRule fs = new EphemeralFileSystemRule();
+ private final PageCacheRule pageCacheRule = new PageCacheRule();
+
+ @Rule
+ public final TestRule rules = RuleChain.outerRule( fs ).around( pageCacheRule );
+
+ private final Queue nextPageId = new ConcurrentLinkedQueue<>();
+ private final Queue nextPageOffset = new ConcurrentLinkedQueue<>();
+ private int intsPerRecord = 1;
+
+ private MyStore store;
+ private Config config = CONFIG;
+
+ @After
+ public void tearDown()
+ {
+ if ( store != null )
+ {
+ store.close();
+ store = null;
+ }
+ nextPageOffset.clear();
+ nextPageId.clear();
+ }
+
+ private void assertThrows( ThrowingAction action ) throws Exception
+ {
+ try
+ {
+ action.apply();
+ fail( "expected an exception" );
+ }
+ catch ( UnderlyingStorageException exception )
+ {
+ // Good!
+ }
+ }
+
+ private void verifyExceptionOnAccess( ThrowingAction access ) throws Exception
+ {
+ createStore();
+ nextPageOffset.add( 8190 );
+ assertThrows( access );
+ }
+
+ private void createStore()
+ {
+ store = new MyStore( config, pageCacheRule.getPageCache( fs.get(), config ) );
+ store.initialise( true );
+ }
+
+ @Test
+ public void writingOfHeaderRecordDuringInitialiseNewStoreFileMustThrowOnPageOverflow() throws Exception
+ {
+ // 16-byte header will overflow an 8-byte page size
+ Config config = CONFIG.with( stringMap( GraphDatabaseSettings.mapped_memory_page_size.name(), "8" ) );
+ MyStore store = new MyStore( config, pageCacheRule.getPageCache( fs.get(), config ) );
+ assertThrows( () -> store.initialise( true ) );
+ }
+
+ @Test
+ public void extractHeaderRecordDuringLoadStorageMustThrowOnPageOverflow() throws Exception
+ {
+ MyStore first = new MyStore( config, pageCacheRule.getPageCache( fs.get(), config ) );
+ first.initialise( true );
+ first.close();
+
+ config = CONFIG.with( stringMap( GraphDatabaseSettings.mapped_memory_page_size.name(), "8" ) );
+ MyStore second = new MyStore( config, pageCacheRule.getPageCache( fs.get(), config ) );
+ assertThrows( () -> second.initialise( false ) );
+ }
+
+ @Test
+ public void getRawRecordDataMustThrowOnPageOverflow() throws Exception
+ {
+ verifyExceptionOnAccess( () -> store.getRawRecordData( 5 ) );
+ }
+
+ @Test
+ public void isInUseMustThrowOnPageOverflow() throws Exception
+ {
+ verifyExceptionOnAccess( () -> store.isInUse( 5 ) );
+ }
+
+ @Test
+ public void getRecordMustThrowOnPageOverflow() throws Exception
+ {
+ verifyExceptionOnAccess( () -> store.getRecord( 5, new IntRecord( 5 ), NORMAL ) );
+ }
+
+ @Test
+ public void updateRecordMustThrowOnPageOverflow() throws Exception
+ {
+ verifyExceptionOnAccess( () -> store.updateRecord( new IntRecord( 5 ) ) );
+ }
+
+ @Test
+ public void recordCursorNextMustThrowOnPageOverflow() throws Exception
+ {
+ verifyExceptionOnAccess( () -> {
+ try ( RecordCursor cursor = store.newRecordCursor( new IntRecord( 0 ) ).acquire( 5, NORMAL ) )
+ {
+ cursor.next();
+ }
+ } );
+ }
+
+ @Test
+ public void rebuildIdGeneratorSlowMustThrowOnPageOverflow() throws Exception
+ {
+ config = config.with( stringMap(
+ CommonAbstractStore.Configuration.rebuild_idgenerators_fast.name(), "false" ) );
+ createStore();
+ store.setStoreNotOk( new Exception() );
+ IntRecord record = new IntRecord( 200 );
+ record.value = 0xCAFEBABE;
+ store.updateRecord( record );
+ intsPerRecord = 8192;
+ assertThrows( () -> store.makeStoreOk() );
+ }
+
+ @Test
+ public void scanForHighIdMustThrowOnPageOverflow() throws Exception
+ {
+ createStore();
+ store.setStoreNotOk( new Exception() );
+ IntRecord record = new IntRecord( 200 );
+ record.value = 0xCAFEBABE;
+ store.updateRecord( record );
+ intsPerRecord = 8192;
+ assertThrows( () -> store.makeStoreOk() );
+ }
+
+ private static class IntRecord extends AbstractBaseRecord
+ {
+ public int value;
+
+ IntRecord( long id )
+ {
+ super( id );
+ setInUse( true );
+ }
+
+ @Override
+ public String toString()
+ {
+ return "IntRecord[" + getId() + "](" + value + ")";
+ }
+ }
+
+ private static class LongLongHeader implements StoreHeader
+ {
+ }
+
+ private class MyFormat extends BaseRecordFormat implements StoreHeaderFormat
+ {
+ MyFormat()
+ {
+ super( (x) -> 4, 8, 32 );
+ }
+
+ @Override
+ public IntRecord newRecord()
+ {
+ return new IntRecord( 0 );
+ }
+
+ @Override
+ public boolean isInUse( PageCursor cursor )
+ {
+ boolean inUse = false;
+ for ( int i = 0; i < intsPerRecord; i++ )
+ {
+ inUse |= cursor.getInt() != 0;
+ }
+ return inUse;
+ }
+
+ @Override
+ public void read( IntRecord record, PageCursor cursor, RecordLoad mode, int recordSize, PagedFile storeFile )
+ throws IOException
+ {
+ for ( int i = 0; i < intsPerRecord; i++ )
+ {
+ record.value = cursor.getInt();
+ }
+ record.setInUse( true );
+ }
+
+ @Override
+ public void write( IntRecord record, PageCursor cursor, int recordSize, PagedFile storeFile ) throws IOException
+ {
+ for ( int i = 0; i < intsPerRecord; i++ )
+ {
+ cursor.putInt( record.value );
+ }
+ }
+
+ @Override
+ public int numberOfReservedRecords()
+ {
+ return 4; // 2 longs occupy 4 int records
+ }
+
+ @Override
+ public void writeHeader( PageCursor cursor )
+ {
+ cursor.putLong( 0xA5A5A5_7E7E7EL );
+ cursor.putLong( 0x3B3B3B_1A1A1AL );
+ }
+
+ @Override
+ public LongLongHeader readHeader( PageCursor cursor )
+ {
+ LongLongHeader header = new LongLongHeader();
+ cursor.getLong(); // pretend to read fields into the header
+ cursor.getLong();
+ return header;
+ }
+ }
+
+ private class MyStore extends CommonAbstractStore
+ {
+ MyStore( Config config, PageCache pageCache )
+ {
+ this( config, pageCache, new MyFormat() );
+ }
+
+ MyStore( Config config, PageCache pageCache, MyFormat format )
+ {
+ super( new File( "store" ), config, IdType.NODE, new DefaultIdGeneratorFactory( fs.get() ), pageCache,
+ NullLogProvider.getInstance(), "T", format, format, "XYZ" );
+ }
+
+ @Override
+ public void accept( Processor processor, IntRecord record )
+ throws FAILURE
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ protected long pageIdForRecord( long id )
+ {
+ Long override = nextPageId.poll();
+ return override != null ? override : super.pageIdForRecord( id );
+ }
+
+ @Override
+ protected int offsetForId( long id )
+ {
+ Integer override = nextPageOffset.poll();
+ return override != null ? override : super.offsetForId( id );
+ }
+ }
+}