From 9234e4b5fca6f929d9b9f60ab14bdd46362a3a57 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Sat, 2 Sep 2017 13:28:53 +0200 Subject: [PATCH] Strengthen error handling of ByteUnit parse And make Settings.BYTES use it. --- .../src/main/java/org/neo4j/io/ByteUnit.java | 18 ++++++++- .../test/java/org/neo4j/io/ByteUnitTest.java | 30 ++++++++++++++ .../neo4j/kernel/configuration/Settings.java | 40 ++++++------------- 3 files changed, 59 insertions(+), 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 index 02c675a623415..272fa5ab8129f 100644 --- 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> 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,12 +234,17 @@ 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 ); } @@ -241,6 +252,11 @@ else if ( !Character.isWhitespace( ch ) ) return result; } + private static IllegalArgumentException invalidFormat( String text ) + { + return new IllegalArgumentException( "Invalid number format: '" + text + "'" ); + } + private static Stream> 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 index d70bc18582421..ed7d65619b7f7 100644 --- 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 index 309e8ad66f3c4..de9eabe9cee9a 100644 --- 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