Strengthen error handling of ByteUnit parse

And make Settings.BYTES use it.
neo4j · Dec 11, 2017 · 9234e4b · 9234e4b
1 parent 9cbd88a
commit 9234e4b
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 29 deletions.
diff --git a/community/io/src/main/java/org/neo4j/io/ByteUnit.java b/community/io/src/main/java/org/neo4j/io/ByteUnit.java
@@ -206,6 +206,7 @@ public static long parse( String text )
         long result = 0;
         int len = text.length();
         int unitCharacter = 0;
+        int digitCharacters = 0;
         Stream<Pair<String,ByteUnit>> unitsStream = listUnits();
 
         for ( int i = 0; i < len; i++ )
@@ -214,11 +215,16 @@ public static long parse( String text )
             int digit = Character.digit( ch, 10 );
             if ( digit != -1 )
             {
+                if ( unitCharacter != 0 )
+                {
+                    throw invalidFormat( text );
+                }
                 if ( result != 0 )
                 {
                     result *= 10;
                 }
                 result += digit;
+                digitCharacters++;
             }
             else if ( !Character.isWhitespace( ch ) )
             {
@@ -228,19 +234,29 @@ else if ( !Character.isWhitespace( ch ) )
             }
         }
 
+        if ( digitCharacters == 0 )
+        {
+            throw invalidFormat( text );
+        }
+
         if ( unitCharacter > 0 )
         {
             ByteUnit byteUnit = unitsStream.map( Pair::other ).findFirst().orElse( null );
             if ( byteUnit == null )
             {
-                throw new IllegalArgumentException( "Unknown byte unit in '" + text + "'" );
+                throw invalidFormat( text );
             }
             result = byteUnit.toBytes( result );
         }
 
         return result;
     }
 
+    private static IllegalArgumentException invalidFormat( String text )
+    {
+        return new IllegalArgumentException( "Invalid number format: '" + text + "'" );
+    }
+
     private static Stream<Pair<String,ByteUnit>> listUnits()
     {
         return Arrays.stream( values() ).flatMap(

diff --git a/community/io/src/test/java/org/neo4j/io/ByteUnitTest.java b/community/io/src/test/java/org/neo4j/io/ByteUnitTest.java
@@ -231,4 +231,34 @@ public void bytesToString()
         assertEquals( "977.5MiB", ByteUnit.bytesToString( 1025000000 ) );
         assertEquals( "9.546GiB", ByteUnit.bytesToString( 10250000000L) );
     }
+
+    @Test( expected = IllegalArgumentException.class )
+    public void mustThrowWhenParsingInvalidUnit() throws Exception
+    {
+        ByteUnit.parse( "1 XB" );
+    }
+
+    @Test( expected = IllegalArgumentException.class )
+    public void mustThrowWhenParsingUnitInterjectedWithNumber() throws Exception
+    {
+        ByteUnit.parse( "1K2i3B" );
+    }
+
+    @Test( expected = IllegalArgumentException.class )
+    public void mustThrowWhenParsingNonNumbericTest() throws Exception
+    {
+        ByteUnit.parse( "abc" );
+    }
+
+    @Test( expected = IllegalArgumentException.class )
+    public void mustThrowWhenParsingOnlyUnit() throws Exception
+    {
+        ByteUnit.parse( "MiB" );
+    }
+
+    @Test( expected = IllegalArgumentException.class )
+    public void mustThrowWhenParsingUnitBeforeValue() throws Exception
+    {
+        ByteUnit.parse( "MiB 1" );
+    }
 }
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/configuration/Settings.java b/community/kernel/src/main/java/org/neo4j/kernel/configuration/Settings.java
@@ -49,6 +49,7 @@
 import org.neo4j.helpers.SocketAddressParser;
 import org.neo4j.helpers.TimeUtil;
 import org.neo4j.helpers.collection.Iterables;
+import org.neo4j.io.ByteUnit;
 
 import static java.lang.Character.isDigit;
 import static java.lang.Double.parseDouble;
@@ -659,39 +660,22 @@ public String valueDescription()
         @Override
         public Long apply( String value )
         {
+            long bytes;
             try
             {
-                String mem = value.toLowerCase();
-                long multiplier = 1;
-                if ( mem.endsWith( "k" ) )
-                {
-                    multiplier = 1024;
-                    mem = mem.substring( 0, mem.length() - 1 );
-                }
-                else if ( mem.endsWith( "m" ) )
-                {
-                    multiplier = 1024 * 1024;
-                    mem = mem.substring( 0, mem.length() - 1 );
-                }
-                else if ( mem.endsWith( "g" ) )
-                {
-                    multiplier = 1024 * 1024 * 1024;
-                    mem = mem.substring( 0, mem.length() - 1 );
-                }
-
-                long bytes = parseLong( mem.trim() ) * multiplier;
-                if ( bytes < 0 )
-                {
-                    throw new IllegalArgumentException(
-                            value + " is not a valid number of bytes. Must be positive or zero." );
-                }
-                return bytes;
+                bytes = ByteUnit.parse( value );
             }
-            catch ( NumberFormatException e )
+            catch ( IllegalArgumentException e )
             {
-                throw new IllegalArgumentException( format( "%s is not a valid size, must be e.g. 10, 5K, 1M, " +
-                        "11G", value ) );
+                throw new IllegalArgumentException( format(
+                        "%s is not a valid size, must be e.g. 10, 5K, 1M, 11G", value ) );
+            }
+            if ( bytes < 0 )
+            {
+                throw new IllegalArgumentException(
+                        value + " is not a valid number of bytes. Must be positive or zero." );
             }
+            return bytes;
         }
 
         @Override