lightbend / config

configuration library for JVM languages using HOCON files
https://lightbend.github.io/config/
6.12k stars 968 forks source link

Inconsistent behavior with Overflow/Underflow and Exceptions #801

Open zleonov opened 7 months ago

zleonov commented 7 months ago

I know this library is feature complete and Lightbend does not plan to make any further changes. But I am working on a project that extends certain functionality of Typesafe Config, and I did a deep dive on the internals of this library.

Posting this here for commentary or for the future community fork if it ever gets off the ground.

It seems that Typesafe Config treats various edge cases in an inconsistent manner.

Consider the following:

// Integer overflow results in ConfigException.WrongType
config = ConfigFactory.parseString("prop = 1" + Integer.MAX_VALUE);
try {
    System.out.println(config.getInt("prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.WrongType.class);
}

// Long overflow results in Long.MAX_VALUE 
config = ConfigFactory.parseString("prop = 1" + Long.MAX_VALUE);
System.out.println(config.getLong("prop") == Long.MAX_VALUE);

// Long overflow for Duration results in ConfigException.WrongType
config = ConfigFactory.parseString("prop = 1" + Long.MAX_VALUE);
try {
    System.out.println(config.getDuration("prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.BadValue.class);
}

// null property results in ConfigException.Null 
config = ConfigFactory.parseString("prop = null");
try {
    System.out.println(config.getDuration("prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.Null.class);
}

// null element in a list results in ConfigException.WrongType 
config = ConfigFactory.parseString("prop = [1, 2, null, 3]");
try {
    System.out.println(config.getIntList("prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.WrongType.class);
}

// bad boolean list results in an error message that DOES NOT explain it's a list
config = ConfigFactory.parseString("prop = [MONDAY, TUESDAY, FOO]");
try {
    System.out.println(config.getEnumList(Day.class, "prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.BadValue.class);

    /* String: 1: Invalid value at 'prop': The enum class Day has no constant of the name
     * 'FOO' (should be one of [MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY].) */
    System.out.println(e.getMessage()); 
}

// bad int list results in an error messaging referencing a list
config = ConfigFactory.parseString("prop = [1, 2, c]");
try {
    System.out.println(config.getIntList("prop"));
} catch (final Exception e) {
    System.out.println(e.getClass() == ConfigException.WrongType.class);
    // String: 1: prop has type list of STRING rather than list of NUMBER
    System.out.println(e.getMessage());
}

This is not an exhaustive list. I understand this may be considered minor issues, but I do think if this code is ever refactored it makes sense to treat all erroneous value conversions the same way:

null values should result in a ConfigException.Null whether they are inside a list or not. Numeric values which underflow or overflow should universally throw the same exception or return the appropriate MAX_VALUE or MIN_VALUE constant, and so on.