Improved error handling in spatial index header reading

neo4j · Apr 4, 2018 · 462d868 · 462d868
1 parent d46f8ff
commit 462d868
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 13 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexFiles.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexFiles.java
@@ -108,7 +108,7 @@ private SpatialFileLayout( CoordinateReferenceSystem crs, SpaceFillingCurveSetti
         public void readHeader( PageCache pageCache ) throws IOException
         {
             GBPTree.readHeader( pageCache, indexFile, settings.headerReader( NativeSchemaIndexHeaderReader::readFailureMessage ) );
-            if ( settings.getFailureMessage() != null )
+            if ( settings.isFailed() )
             {
                 throw new IOException( settings.getFailureMessage() );
             }

diff --git a/...el/src/main/java/org/neo4j/kernel/impl/index/schema/config/SpaceFillingCurveSettings.java b/...el/src/main/java/org/neo4j/kernel/impl/index/schema/config/SpaceFillingCurveSettings.java
@@ -19,6 +19,7 @@
  */
 package org.neo4j.kernel.impl.index.schema.config;
 
+import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.function.Consumer;
@@ -117,6 +118,26 @@ public String getFailureMessage()
         return failureMessage;
     }
 
+    /**
+     * The settings are read from the GBPTree header structure, but when this is a FAILED index, there are no settings, but instead an error message
+     * describing the failure. If that happens, code that triggered the read should check this. If the value is true, calling getFailureMessage() will
+     * provide an error message describing the failure.
+     */
+    public boolean isFailed()
+    {
+        return this.failureMessage != null;
+    }
+
+    private void markAsFailed( String failureMessage )
+    {
+        this.failureMessage = failureMessage;
+    }
+
+    private void markAsSucceeded()
+    {
+        this.failureMessage = null;
+    }
+
     /**
      * Make an instance of the SpaceFillingCurve that can perform the 2D (or 3D) to 1D mapping based on these settings.
      *
@@ -166,16 +187,23 @@ public void writeHeader( SpaceFillingCurveSettings settings, PageCursor cursor )
                     @Override
                     public void readHeader( SpaceFillingCurveSettings settings, ByteBuffer headerBytes )
                     {
-                        settings.maxLevels = headerBytes.getInt();
-                        settings.dimensions = headerBytes.getInt();
-                        double[] min = new double[settings.dimensions];
-                        double[] max = new double[settings.dimensions];
-                        for ( int i = 0; i < settings.dimensions; i++ )
+                        try
                         {
-                            min[i] = headerBytes.getDouble();
-                            max[i] = headerBytes.getDouble();
+                            settings.maxLevels = headerBytes.getInt();
+                            settings.dimensions = headerBytes.getInt();
+                            double[] min = new double[settings.dimensions];
+                            double[] max = new double[settings.dimensions];
+                            for ( int i = 0; i < settings.dimensions; i++ )
+                            {
+                                min[i] = headerBytes.getDouble();
+                                max[i] = headerBytes.getDouble();
+                            }
+                            settings.extents = new Envelope( min, max );
+                        }
+                        catch ( BufferUnderflowException e )
+                        {
+                            settings.markAsFailed( "Failed to read settings from GBPTree header: " + e.getMessage() );
                         }
-                        settings.extents = new Envelope( min, max );
                     }
                 };
         private int id;
@@ -223,14 +251,17 @@ public Header.Reader headerReader( Function<ByteBuffer,String> onError )
             }
             else
             {
-                this.failureMessage = null;
                 int typeId = headerBytes.getInt();
                 SpatialIndexType indexType = SpatialIndexType.get( typeId );
                 if ( indexType == null )
                 {
-                    this.failureMessage = "Unknown spatial index type in index header: " + typeId;
+                    markAsFailed( "Unknown spatial index type in index header: " + typeId );
+                }
+                else
+                {
+                    markAsSucceeded();
+                    indexType.readHeader( SpaceFillingCurveSettings.this, headerBytes );
                 }
-                indexType.readHeader( SpaceFillingCurveSettings.this, headerBytes );
             }
         };
     }

diff --git a/...main/java/org/neo4j/kernel/impl/index/schema/config/SpaceFillingCurveSettingsFactory.java b/...main/java/org/neo4j/kernel/impl/index/schema/config/SpaceFillingCurveSettingsFactory.java
@@ -77,7 +77,7 @@ public SpaceFillingCurveSettingsFactory( Config config )
             if ( key.startsWith( SPATIAL_SETTING_PREFIX ) )
             {
                 String[] fields = key.replace( SPATIAL_SETTING_PREFIX, "" ).split( "\\." );
-                if ( fields.length < 3 )
+                if ( fields.length != 3 )
                 {
                     throw new IllegalArgumentException(
                             "Invalid spatial config settings, expected three fields after '" + SPATIAL_SETTING_PREFIX + "': " + key );
@@ -91,6 +91,10 @@ public SpaceFillingCurveSettingsFactory( Config config )
                     {
                         throw new IllegalArgumentException( "Invalid spatial coordinate key (should be one of 'x', 'y' or 'z'): " + fields[1] );
                     }
+                    if ( index >= crs.getDimension() )
+                    {
+                        throw new IllegalArgumentException( "Invalid spatial coordinate key for " + crs.getDimension() + "D: " + fields[1] );
+                    }
                     switch ( fields[2].toLowerCase() )
                     {
                     case "min":