AFS to only gunk up part of byte[] it has access to

Before this commit, AdversarialFileSystem will inject random data into the complete array given to `getBytes(..)`; meaning a call like `getBytes(bigBuffer, 1000, 10)` could lead to the entire buffer being messed up, rather than just the range the user has asked to read into. This changes the AFS to only "mess up" the section of the array that would actually be written to by Muninn.
neo4j · Jan 8, 2018 · 0c06803 · 0c06803
1 parent abede69
commit 0c06803
Show file tree

Hide file tree

Showing 13 changed files with 149 additions and 35 deletions.
diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/FreelistNodeTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/FreelistNodeTest.java
@@ -21,6 +21,7 @@
 
 import org.junit.Test;
 
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
 import org.neo4j.io.pagecache.PageCursor;
 
 import static org.junit.Assert.assertEquals;

diff --git a/...y/index/src/test/java/org/neo4j/index/internal/gbptree/GenerationSafePointerPairTest.java b/...y/index/src/test/java/org/neo4j/index/internal/gbptree/GenerationSafePointerPairTest.java
@@ -28,6 +28,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
 import org.neo4j.io.pagecache.PageCursor;
 
 import static org.hamcrest.CoreMatchers.containsString;

diff --git a/...unity/index/src/test/java/org/neo4j/index/internal/gbptree/GenerationSafePointerTest.java b/...unity/index/src/test/java/org/neo4j/index/internal/gbptree/GenerationSafePointerTest.java
@@ -22,6 +22,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.test.rule.RandomRule;
 

diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/KeySearchTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/KeySearchTest.java
@@ -30,7 +30,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
-import static org.neo4j.index.internal.gbptree.ByteArrayPageCursor.wrap;
+import static org.neo4j.io.pagecache.ByteArrayPageCursor.wrap;
 import static org.neo4j.index.internal.gbptree.KeySearch.search;
 
 public class KeySearchTest

diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/PageAwareByteArrayCursor.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/PageAwareByteArrayCursor.java
@@ -27,7 +27,7 @@
 import org.neo4j.io.pagecache.CursorException;
 import org.neo4j.io.pagecache.PageCursor;
 
-import static org.neo4j.index.internal.gbptree.ByteArrayPageCursor.wrap;
+import static org.neo4j.io.pagecache.ByteArrayPageCursor.wrap;
 
 class PageAwareByteArrayCursor extends PageCursor
 {

diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/PageCursorUtilTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/PageCursorUtilTest.java
@@ -22,6 +22,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.test.rule.RandomRule;
 

diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/PointerCheckingTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/PointerCheckingTest.java
@@ -21,6 +21,7 @@
 
 import org.junit.Test;
 
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
 import org.neo4j.io.pagecache.PageCursor;
 
 import static org.junit.Assert.fail;

diff --git a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java
@@ -60,8 +60,7 @@
 @SuppressWarnings( "unchecked" )
 class AdversarialReadPageCursor extends DelegatingPageCursor
 {
-    private static final boolean enableInconsistencyTracing = FeatureToggles.flag(
-            AdversarialReadPageCursor.class, "enableInconsistencyTracing", false );
+    private static final boolean enableInconsistencyTracing = FeatureToggles.flag( AdversarialReadPageCursor.class, "enableInconsistencyTracing", false );
 
     private static class State implements Adversary
     {
@@ -106,15 +105,17 @@ private <T extends Number> Number inconsistently( T value, PageCursor delegate )
             return value;
         }
 
-        private void inconsistently( byte[] data )
+        private void inconsistently( byte[] data, int arrayOffset, int length )
         {
             if ( currentReadIsPreparingInconsistent )
             {
                 callCounter++;
             }
             else if ( currentReadIsInconsistent )
             {
-                ThreadLocalRandom.current().nextBytes( data );
+                byte[] gunk = new byte[length];
+                ThreadLocalRandom.current().nextBytes( gunk );
+                System.arraycopy( gunk, 0, data, arrayOffset, length );
                 inconsistentReadHistory.add( Arrays.copyOf( data, data.length ) );
             }
         }
@@ -173,7 +174,7 @@ public boolean isInconsistent()
     private AdversarialReadPageCursor linkedCursor;
     private final State state;
 
-    AdversarialReadPageCursor( PageCursor delegate, Adversary adversary )
+    public AdversarialReadPageCursor( PageCursor delegate, Adversary adversary )
     {
         super( delegate );
         this.state = new State( Objects.requireNonNull( adversary ) );
@@ -196,9 +197,9 @@ private <T extends Number> Number inconsistently( T value )
         return state.inconsistently( value, delegate );
     }
 
-    private void inconsistently( byte[] data )
+    private void inconsistently( byte[] data, int arrayOffset, int length )
     {
-        state.inconsistently( data );
+        state.inconsistently( data, arrayOffset, length );
     }
 
     @Override
@@ -271,14 +272,14 @@ public void putInt( int offset, int value )
     public void getBytes( byte[] data )
     {
         delegate.getBytes( data );
-        inconsistently( data );
+        inconsistently( data, 0, data.length );
     }
 
     @Override
     public void getBytes( byte[] data, int arrayOffset, int length )
     {
         delegate.getBytes( data, arrayOffset, length );
-        inconsistently( data );
+        inconsistently( data, arrayOffset, length );
     }
 
     @Override
@@ -370,8 +371,8 @@ public boolean next( long pageId ) throws IOException
 
     private void prepareNext()
     {
-        boolean currentReadIsPreparingInconsistent = state.injectFailureOrMischief( FileNotFoundException.class, IOException.class,
-                SecurityException.class, IllegalStateException.class );
+        boolean currentReadIsPreparingInconsistent =
+                state.injectFailureOrMischief( FileNotFoundException.class, IOException.class, SecurityException.class, IllegalStateException.class );
         state.reset( currentReadIsPreparingInconsistent );
     }
 
@@ -385,8 +386,7 @@ public void close()
     @Override
     public boolean shouldRetry() throws IOException
     {
-        state.injectFailure( FileNotFoundException.class, IOException.class, SecurityException.class,
-                IllegalStateException.class );
+        state.injectFailure( FileNotFoundException.class, IOException.class, SecurityException.class, IllegalStateException.class );
         if ( state.hasPreparedInconsistentRead() )
         {
             resetDelegate();

diff --git a/...unity/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursorTest.java b/...unity/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursorTest.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2002-2018 "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 <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.adversaries.pagecache;
+
+import org.junit.Test;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.neo4j.io.pagecache.ByteArrayPageCursor;
+import org.neo4j.test.rule.PageCacheRule;
+
+import static org.junit.Assert.assertEquals;
+
+public class AdversarialReadPageCursorTest
+{
+    @Test
+    public void shouldNotMessUpUnrelatedSegmentOnReadBytes() throws Exception
+    {
+        // Given
+        byte[] buf = new byte[4];
+        byte[] page = new byte[]{7};
+        AdversarialReadPageCursor cursor = new AdversarialReadPageCursor( new ByteArrayPageCursor( page ),
+                new PageCacheRule.AtomicBooleanInconsistentReadAdversary( new AtomicBoolean( true ) ) );
+
+        // When
+        cursor.next( 0 );
+        cursor.getBytes( buf, buf.length - 1, 1 );
+        cursor.shouldRetry();
+        cursor.getBytes( buf, buf.length - 1, 1 );
+
+        // Then the range outside of buf.length-1, buf.length should be pristine
+        assertEquals( 0, buf[0] );
+        assertEquals( 0, buf[1] );
+        assertEquals( 0, buf[2] );
+    }
+}
diff --git a/...internal/gbptree/ByteArrayPageCursor.java → ...o4j/io/pagecache/ByteArrayPageCursor.java b/...internal/gbptree/ByteArrayPageCursor.java → ...o4j/io/pagecache/ByteArrayPageCursor.java
@@ -17,52 +17,48 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-package org.neo4j.index.internal.gbptree;
+package org.neo4j.io.pagecache;
 
 import java.io.File;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 
 import org.neo4j.helpers.Exceptions;
-import org.neo4j.io.pagecache.CursorException;
-import org.neo4j.io.pagecache.PageCursor;
 
 /**
  * Wraps a byte array and present it as a PageCursor.
  * <p>
- * This class is bridging something which would otherwise make {@link InternalTreeLogic} code slightly more
- * complicated. Currently when splitting nodes keys/values/children are read into temporary arrays
- * and manipulating that data by standard means (which are designed to work on {@link PageCursor}
- * can stay the same if wrapping the byte array as such. If splitting code later changes to not
- * do this temporary copy then this class won't be needed anymore.
- * <p>
  * All the accessor methods (getXXX, putXXX) are implemented and delegates calls to its internal {@link ByteBuffer}.
  * {@link #setOffset(int)}, {@link #getOffset()} and {@link #rewind()} positions the internal {@link ByteBuffer}.
- * {@link #shouldRetry()} always returns {@code false}. No other methods should be used and will throw
- * {@link UnsupportedOperationException}.
+ * {@link #shouldRetry()} always returns {@code false}.
  */
-class ByteArrayPageCursor extends PageCursor
+public class ByteArrayPageCursor extends PageCursor
 {
     private final ByteBuffer buffer;
     private CursorException cursorException;
 
-    static PageCursor wrap( byte[] array, int offset, int length )
+    public static PageCursor wrap( byte[] array, int offset, int length )
     {
         return new ByteArrayPageCursor( array, offset, length );
     }
 
-    static PageCursor wrap( byte[] array )
+    public static PageCursor wrap( byte[] array )
     {
         return wrap( array, 0, array.length );
     }
 
-    static PageCursor wrap( int length )
+    public static PageCursor wrap( int length )
     {
         return wrap( new byte[length] );
     }
 
-    private ByteArrayPageCursor( byte[] array, int offset, int length )
+    public ByteArrayPageCursor( byte[] array )
+    {
+        this( array, 0, array.length );
+    }
+
+    public ByteArrayPageCursor( byte[] array, int offset, int length )
     {
         this.buffer = ByteBuffer.wrap( array, offset, length );
     }
@@ -232,7 +228,7 @@ public boolean next() throws IOException
     @Override
     public boolean next( long pageId ) throws IOException
     {
-        throw new UnsupportedOperationException();
+        return pageId == 0;
     }
 
     @Override

diff --git a/community/io/src/test/java/org/neo4j/test/impl/ChannelOutputStream.java b/community/io/src/test/java/org/neo4j/test/impl/ChannelOutputStream.java
@@ -61,7 +61,7 @@ public void write( byte[] b, int off, int len ) throws IOException
         while ( written < len )
         {
             buffer.clear();
-            buffer.put( b, index, Math.min( len - written, buffer.capacity() ) );
+            buffer.put( b, index + written, Math.min( len - written, buffer.capacity() ) );
             buffer.flip();
             written += channel.write( buffer );
         }

diff --git a/community/io/src/test/java/org/neo4j/test/impl/ChannelOutputStreamTest.java b/community/io/src/test/java/org/neo4j/test/impl/ChannelOutputStreamTest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2002-2018 "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 <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.test.impl;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import org.neo4j.test.rule.fs.EphemeralFileSystemRule;
+import org.neo4j.test.rule.fs.FileSystemRule;
+
+public class ChannelOutputStreamTest
+{
+    @Rule
+    public FileSystemRule fs = new EphemeralFileSystemRule();
+
+    @Rule
+    public TemporaryFolder tmpDir = new TemporaryFolder();
+
+    @Test
+    public void shouldStoreAByteAtBoundary() throws Exception
+    {
+        File workFile = tmpDir.newFile();
+        fs.mkdirs( tmpDir.getRoot() );
+        OutputStream out = fs.openAsOutputStream( workFile, false );
+
+        // When I write a byte[] that is larger than the internal buffer in
+        // ChannelOutputStream..
+        byte[] b = new byte[8097];
+        b[b.length - 1] = 7;
+        out.write( b );
+        out.flush();
+
+        // Then it should get cleanly written and be readable
+        InputStream in = fs.openAsInputStream( workFile );
+        in.skip( 8096 );
+        assert in.read() == 7;
+    }
+}
diff --git a/community/io/src/test/java/org/neo4j/test/rule/PageCacheRule.java b/community/io/src/test/java/org/neo4j/test/rule/PageCacheRule.java
@@ -231,11 +231,11 @@ protected void after( boolean success )
         }
     }
 
-    private static class AtomicBooleanInconsistentReadAdversary implements Adversary
+    public static class AtomicBooleanInconsistentReadAdversary implements Adversary
     {
         final AtomicBoolean nextReadIsInconsistent;
 
-        AtomicBooleanInconsistentReadAdversary( AtomicBoolean nextReadIsInconsistent )
+        public AtomicBooleanInconsistentReadAdversary( AtomicBoolean nextReadIsInconsistent )
         {
             this.nextReadIsInconsistent = nextReadIsInconsistent;
         }