fixed contract violation which stems from incorect exception thrown when reading bytes and memorysize values

config/src/main/java/com/typesafe/config/ConfigMemorySize.java

@@ -17,7 +17,7 @@ public final class ConfigMemorySize {
     private BigInteger bytes;
 
     private ConfigMemorySize(BigInteger bytes) {
-        if (bytes.compareTo(BigInteger.ZERO) < 0)
+        if (bytes.signum() < 0)
             throw new IllegalArgumentException("Attempt to construct ConfigMemorySize with negative number: " + bytes);
         this.bytes = bytes;
     }
@@ -48,6 +48,9 @@ public final class ConfigMemorySize {
      *
      * @since 1.3.0
      * @return how many bytes
+     * @exception IllegalArgumentException when memory value
+     * in bytes doesn't fit in a long value. Consider using
+     * {@link #toBytesBigInteger} in this case.
      */
     public long toBytes() {
         if (bytes.bitLength() < 64)

config/src/main/java/com/typesafe/config/impl/SimpleConfig.java

@@ -283,20 +283,54 @@ final class SimpleConfig implements Config, MergeableValue, Serializable {
 
     @Override
     public Long getBytes(String path) {
-        return getMemorySize(path).toBytes();
+        BigInteger bytes = getBytesBigInteger(path);
+        ConfigValue v = find(path, ConfigValueType.STRING);
+        return toLong(bytes,v.origin(), path);
+    }
+
+    private BigInteger getBytesBigInteger(String path) {
+        BigInteger bytes;
+        ConfigValue v = find(path, ConfigValueType.STRING);
+        try {
+            bytes = BigInteger.valueOf(getLong(path));
+        } catch (ConfigException.WrongType e) {
+            bytes = parseBytes((String) v.unwrapped(),
+                v.origin(), path);
+        }
+        if (bytes.signum() < 0)
+            throw new ConfigException.BadValue(v.origin(), path,
+                "Attempt to construct memory size with negative number: " + bytes);
+        return bytes;
+    }
+
+    private List<BigInteger> getBytesListBigInteger(String path){
+        List<BigInteger> result = new ArrayList<>();
+        List<? extends ConfigValue> list = getList(path);
+
+        for (ConfigValue v : list) {
+            BigInteger bytes;
+            if (v.valueType() == ConfigValueType.NUMBER) {
+                bytes = BigInteger.valueOf(((Number) v.unwrapped()).longValue());
+            } else if (v.valueType() == ConfigValueType.STRING) {
+                String s = (String) v.unwrapped();
+                bytes = parseBytes(s, v.origin(), path);
+            } else {
+                throw new ConfigException.WrongType(v.origin(), path,
+                    "memory size string or number of bytes", v.valueType()
+                    .name());
+            }
+            if (bytes.signum() < 0)
+                throw new ConfigException.BadValue(v.origin(), path,
+                    "Attempt to construct ConfigMemorySize with negative number: " + bytes);
+
+            result.add(bytes);
+        }
+        return result;
     }
 
     @Override
     public ConfigMemorySize getMemorySize(String path) {
-        BigInteger size;
+        return ConfigMemorySize.ofBytes(getBytesBigInteger(path));
-        try {
-            size = BigInteger.valueOf(getLong(path));
-        } catch (ConfigException.WrongType e) {
-            ConfigValue v = find(path, ConfigValueType.STRING);
-            size = parseBytesAsBigInteger((String) v.unwrapped(),
-                v.origin(), path);
-        }
-        return ConfigMemorySize.ofBytes(size);
     }
 
     @Deprecated
@@ -483,29 +517,27 @@ final class SimpleConfig implements Config, MergeableValue, Serializable {
 
     @Override
     public List<Long> getBytesList(String path) {
-        return getMemorySizeList(path).stream()
+        ConfigValue v = find(path, ConfigValueType.LIST);
-            .map(ConfigMemorySize::toBytes)
+        return getBytesListBigInteger(path).stream()
+            .map(bytes -> toLong(bytes, v.origin(), path))
             .collect(Collectors.toList());
     }
 
+    private Long toLong(BigInteger value, ConfigOrigin originForException,
+        String pathForException){
+        if (value.bitLength() < 64) {
+            return value.longValue();
+        } else {
+            throw new ConfigException.BadValue(originForException, pathForException,
+                "size-in-bytes value is out of range for a 64-bit long: '" + value + "'");
+        }
+    }
+
     @Override
     public List<ConfigMemorySize> getMemorySizeList(String path) {
-        List<ConfigMemorySize> l = new ArrayList<>();
+        return getBytesListBigInteger(path).stream()
-        List<? extends ConfigValue> list = getList(path);
+            .map(ConfigMemorySize::ofBytes)
-        for (ConfigValue v : list) {
+            .collect(Collectors.toList());
-            if (v.valueType() == ConfigValueType.NUMBER) {
-                l.add(ConfigMemorySize.ofBytes(((Number) v.unwrapped()).longValue()));
-            } else if (v.valueType() == ConfigValueType.STRING) {
-                String s = (String) v.unwrapped();
-                BigInteger n = parseBytesAsBigInteger(s, v.origin(), path);
-                l.add(ConfigMemorySize.ofBytes(n));
-            } else {
-                throw new ConfigException.WrongType(v.origin(), path,
-                        "memory size string or number of bytes", v.valueType()
-                                .name());
-            }
-        }
-        return l;
     }
 
     @Override
@@ -846,18 +878,7 @@ final class SimpleConfig implements Config, MergeableValue, Serializable {
      * @throws ConfigException
      *             if string is invalid
      */
-    public static long parseBytes(String input, ConfigOrigin originForException,
+    public static BigInteger parseBytes(String input, ConfigOrigin originForException,
-            String pathForException) {
-        BigInteger result = parseBytesAsBigInteger(input, originForException, pathForException);
-        if (result.bitLength() < 64)
-            return result.longValue();
-        else
-            throw new ConfigException.BadValue(originForException, pathForException,
-                "size-in-bytes value is out of range for a 64-bit long: '" + input + "'");
-    }
-
-
-    public static BigInteger parseBytesAsBigInteger(String input, ConfigOrigin originForException,
         String pathForException) {
         String s = ConfigImplUtil.unicodeTrim(input);
         String unitString = getUnits(s);

config/src/test/scala/com/typesafe/config/impl/UnitParserTest.scala

@@ -3,6 +3,7 @@
  */
 package com.typesafe.config.impl
 
+import java.math.BigInteger
 import java.time.{ LocalDate, Period }
 import java.time.temporal.ChronoUnit
 
@@ -89,10 +90,10 @@ class UnitParserTest extends TestUtils {
 
     @Test
     def parseMemorySizeInBytes(): Unit = {
-        def parseMem(s: String): Long = SimpleConfig.parseBytes(s, fakeOrigin(), "test")
+        def parseMem(s: String): BigInteger = SimpleConfig.parseBytes(s, fakeOrigin(), "test")
 
-        assertEquals(Long.MaxValue, parseMem(s"${Long.MaxValue} bytes"))
+        assertEquals(BigInteger.valueOf(Long.MaxValue), parseMem(s"${Long.MaxValue} bytes"))
-        assertEquals(Long.MinValue, parseMem(s"${Long.MinValue} bytes"))
+        assertEquals(BigInteger.valueOf(Long.MinValue), parseMem(s"${Long.MinValue} bytes"))
 
         val oneMebis = List("1048576", "1048576b", "1048576bytes", "1048576byte",
             "1048576  b", "1048576  bytes",
@@ -104,7 +105,7 @@ class UnitParserTest extends TestUtils {
 
         for (s <- oneMebis) {
             val result = parseMem(s)
-            assertEquals(1024 * 1024, result)
+            assertEquals(BigInteger.valueOf(1024 * 1024), result)
         }
 
         val oneMegas = List("1000000", "1000000b", "1000000bytes", "1000000byte",
@@ -117,13 +118,13 @@ class UnitParserTest extends TestUtils {
 
         for (s <- oneMegas) {
             val result = parseMem(s)
-            assertEquals(1000 * 1000, result)
+            assertEquals(BigInteger.valueOf(1000 * 1000), result)
         }
 
-        var result = 1024L * 1024 * 1024
+        var result = BigInteger.valueOf(1024L * 1024 * 1024)
-        for (unit <- Seq("tebi", "pebi", "exbi")) {
+        for (unit <- Seq("tebi", "pebi", "exbi", "zebi", "yobi")) {
             val first = unit.substring(0, 1).toUpperCase()
-            result = result * 1024
+            result = result.multiply(BigInteger.valueOf(1024))
             assertEquals(result, parseMem("1" + first))
             assertEquals(result, parseMem("1" + first + "i"))
             assertEquals(result, parseMem("1" + first + "iB"))
@@ -131,10 +132,10 @@ class UnitParserTest extends TestUtils {
             assertEquals(result, parseMem("1" + unit + "bytes"))
         }
 
-        result = 1000L * 1000 * 1000
+        result = BigInteger.valueOf(1000L * 1000 * 1000)
-        for (unit <- Seq("tera", "peta", "exa")) {
+        for (unit <- Seq("tera", "peta", "exa", "zetta", "yotta")) {
             val first = unit.substring(0, 1).toUpperCase()
-            result = result * 1000
+            result = result.multiply(BigInteger.valueOf(1000))
             assertEquals(result, parseMem("1" + first + "B"))
             assertEquals(result, parseMem("1" + unit + "byte"))
             assertEquals(result, parseMem("1" + unit + "bytes"))
@@ -156,7 +157,7 @@ class UnitParserTest extends TestUtils {
     // later on we'll want to check this with BigInteger version of getBytes
     @Test
     def parseHugeMemorySizes(): Unit = {
-        def parseMem(s: String): Long = SimpleConfig.parseBytes(s, fakeOrigin(), "test")
+        def parseMem(s: String): Long = ConfigFactory.parseString(s"v = $s").getBytes("v")
         def assertOutOfRange(s: String): Unit = {
             val fail = intercept[ConfigException.BadValue] {
                 parseMem(s)
@@ -164,11 +165,17 @@ class UnitParserTest extends TestUtils {
             assertTrue("number was too big", fail.getMessage.contains("out of range"))
         }
 
+        def assertNegativeNumber(s: String): Unit = {
+            val fail = intercept[ConfigException.BadValue] {
+                parseMem(s)
+            }
+            assertTrue("number was negative", fail.getMessage.contains("negative number"))
+        }
+
         import java.math.BigInteger
         assertOutOfRange(s"${BigInteger.valueOf(Long.MaxValue).add(BigInteger.valueOf(1)).toString} bytes")
-        assertOutOfRange(s"${BigInteger.valueOf(Long.MinValue).subtract(BigInteger.valueOf(1)).toString} bytes")
+        assertNegativeNumber(s"${BigInteger.valueOf(Long.MinValue).subtract(BigInteger.valueOf(1)).toString} bytes")
 
-        var result = 1024L * 1024 * 1024
         for (unit <- Seq("zebi", "yobi")) {
             val first = unit.substring(0, 1).toUpperCase()
             assertOutOfRange("1" + first)
@@ -177,17 +184,16 @@ class UnitParserTest extends TestUtils {
             assertOutOfRange("1" + unit + "byte")
             assertOutOfRange("1" + unit + "bytes")
             assertOutOfRange("1.1" + first)
-            assertOutOfRange("-1" + first)
+            assertNegativeNumber("-1" + first)
         }
 
-        result = 1000L * 1000 * 1000
         for (unit <- Seq("zetta", "yotta")) {
             val first = unit.substring(0, 1).toUpperCase()
             assertOutOfRange("1" + first + "B")
             assertOutOfRange("1" + unit + "byte")
             assertOutOfRange("1" + unit + "bytes")
             assertOutOfRange("1.1" + first + "B")
-            assertOutOfRange("-1" + first + "B")
+            assertNegativeNumber("-1" + first + "B")
         }
 
         assertOutOfRange("1000 exabytes")