diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java index b081b704369..742c0853a7b 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java @@ -21,6 +21,16 @@ final class ConfigConverter { private static final Logger log = LoggerFactory.getLogger(ConfigConverter.class); + /** + * Custom exception for invalid boolean values to maintain backward compatibility. When caught in + * ConfigProvider, boolean methods should return false instead of default value. + */ + static class InvalidBooleanValueException extends IllegalArgumentException { + public InvalidBooleanValueException(String message) { + super(message); + } + } + private static final ValueOfLookup LOOKUP = new ValueOfLookup(); /** @@ -39,6 +49,8 @@ static T valueOf(final String value, @Nonnull final Class tClass) { return (T) LOOKUP.get(tClass).invoke(value); } catch (final NumberFormatException e) { throw e; + } catch (final IllegalArgumentException e) { + throw e; } catch (final Throwable e) { log.debug("Can't parse: ", e); throw new NumberFormatException(e.toString()); @@ -413,8 +425,15 @@ static BitSet parseIntegerRangeSet(@Nonnull String str, final String settingName public static Boolean booleanValueOf(String value) { if ("1".equals(value)) { return Boolean.TRUE; + } else if ("0".equals(value)) { + return Boolean.FALSE; + } else if ("true".equalsIgnoreCase(value)) { + return Boolean.TRUE; + } else if ("false".equalsIgnoreCase(value)) { + return Boolean.FALSE; } else { - return Boolean.valueOf(value); + // Throw custom exception for invalid boolean values to maintain backward compatibility + throw new InvalidBooleanValueException("Invalid boolean value: " + value); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 1edeb072c80..2e44d730d54 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -193,8 +193,8 @@ public double getDouble(String key, double defaultValue) { private T get(String key, T defaultValue, Class type, String... aliases) { for (ConfigProvider.Source source : sources) { + String sourceValue = source.get(key, aliases); try { - String sourceValue = source.get(key, aliases); T value = ConfigConverter.valueOf(sourceValue, type); if (value != null) { if (collectConfig) { @@ -202,8 +202,18 @@ private T get(String key, T defaultValue, Class type, String... aliases) } return value; } - } catch (NumberFormatException ex) { - // continue + } catch (ConfigConverter.InvalidBooleanValueException ex) { + // For backward compatibility: invalid boolean values should return false, not default + // Store the invalid sourceValue for telemetry, but return false for the application + if (Boolean.class.equals(type)) { + if (collectConfig) { + ConfigCollector.get().put(key, sourceValue, source.origin()); + } + return (T) Boolean.FALSE; + } + // For non-boolean types, continue to next source + } catch (IllegalArgumentException ex) { + // continue - covers both NumberFormatException and other IllegalArgumentException } } if (collectConfig) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 2905470f5c5..0a7f1a5e7b3 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -2042,17 +2042,12 @@ class ConfigTest extends DDSpecification { where: // spotless:off value | tClass | expected - "42.42" | Boolean | false - "42.42" | Boolean | false "true" | Boolean | true "trUe" | Boolean | true - "trUe" | Boolean | true - "tru" | Boolean | false - "truee" | Boolean | false - "true " | Boolean | false - " true" | Boolean | false - " true " | Boolean | false - " true " | Boolean | false + "false" | Boolean | false + "False" | Boolean | false + "1" | Boolean | true + "0" | Boolean | false "42.42" | Float | 42.42f "42.42" | Double | 42.42 "44" | Integer | 44 @@ -2079,6 +2074,20 @@ class ConfigTest extends DDSpecification { // spotless:on } + def "valueOf negative test for invalid boolean values"() { + when: + ConfigConverter.valueOf(value, Boolean) + + then: + def exception = thrown(IllegalArgumentException) + exception.message.contains("Invalid boolean value") + + where: + // spotless:off + value << ["42.42", "tru", "truee", "true ", " true", " true ", " true ", "notABool", "invalid", "yes", "no", "42"] + // spotless:on + } + def "valueOf negative test"() { when: ConfigConverter.valueOf(value, tClass) diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy index a914aa0cc14..0f9e1dd752d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy @@ -23,6 +23,31 @@ class ConfigConverterTest extends DDSpecification { "0" | false } + def "Convert boolean properties throws exception for invalid values"() { + when: + ConfigConverter.valueOf(invalidValue, Boolean) + + then: + def exception = thrown(ConfigConverter.InvalidBooleanValueException) + exception.message.contains("Invalid boolean value:") + + where: + invalidValue << [ + "42.42", + "tru", + "truee", + "true ", + " true", + " true ", + " true ", + "notABool", + "yes", + "no", + "on", + "off" + ] + } + def "parse map properly for #mapString"() { when: def result = ConfigConverter.parseMap(mapString, "test")