From 9d5ded45afc7d30379f9e8fca09ab46f509da253 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 15 Aug 2025 14:42:49 -0400 Subject: [PATCH 1/5] throw IllegalArgumentException in ConfigConverter.booleanValueOf --- .../config/provider/ConfigConverter.java | 11 +++++++- .../config/provider/ConfigProvider.java | 2 +- .../provider/ConfigConverterTest.groovy | 25 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) 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..ca9219909c8 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 @@ -39,6 +39,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 +415,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 exception for invalid boolean values + throw new IllegalArgumentException("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..a53a92a5e70 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 @@ -202,7 +202,7 @@ private T get(String key, T defaultValue, Class type, String... aliases) } return value; } - } catch (NumberFormatException ex) { + } catch (NumberFormatException | IllegalArgumentException ex) { // continue } } 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..4515f2f5d70 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(IllegalArgumentException) + 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") From 10407d3bfda2d49b083a37b99062081970e0732c Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 15 Aug 2025 14:50:55 -0400 Subject: [PATCH 2/5] modify get to only catch IllegalArgumentException --- .../trace/bootstrap/config/provider/ConfigProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 a53a92a5e70..211290056f1 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 @@ -202,8 +202,8 @@ private T get(String key, T defaultValue, Class type, String... aliases) } return value; } - } catch (NumberFormatException | IllegalArgumentException ex) { - // continue + } catch (IllegalArgumentException ex) { + // continue - covers both NumberFormatException and IllegalArgumentException } } if (collectConfig) { From f0c322e079a20f8d01bbfed9668a8ec19ec78ddd Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 19 Aug 2025 14:50:45 -0400 Subject: [PATCH 3/5] Fix failing ConfigTest tests --- .../decorator/BaseDecorator.java | 2 +- .../datadog/trace/api/ConfigTest.groovy | 29 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index bb575d92080..0d081bc95ca 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -42,7 +42,7 @@ public String apply(Class clazz) { protected final boolean traceAnalyticsEnabled; protected final Double traceAnalyticsSampleRate; - protected BaseDecorator() { + protected BaseDeqcorator() { final Config config = Config.get(); final String[] instrumentationNames = instrumentationNames(); this.traceAnalyticsEnabled = 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..ef4c13c6211 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -657,7 +657,7 @@ class ConfigTest extends DDSpecification { config.agentHost == " " config.agentPort == 8126 config.agentUrl == "http:// :8126" - config.prioritySamplingEnabled == false + config.prioritySamplingEnabled == true config.traceResolverEnabled == true config.serviceMapping == [:] config.mergedSpanTags == [:] @@ -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) From fdb60f8875034348f1a02d64eb6f820449b802c3 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Tue, 19 Aug 2025 16:38:01 -0400 Subject: [PATCH 4/5] Update dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java Co-authored-by: Stuart McCulloch --- .../bootstrap/instrumentation/decorator/BaseDecorator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index 0d081bc95ca..bb575d92080 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -42,7 +42,7 @@ public String apply(Class clazz) { protected final boolean traceAnalyticsEnabled; protected final Double traceAnalyticsSampleRate; - protected BaseDeqcorator() { + protected BaseDecorator() { final Config config = Config.get(); final String[] instrumentationNames = instrumentationNames(); this.traceAnalyticsEnabled = From 241ba693cceb4bf13c5591bf4982320d2220f6a4 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 20 Aug 2025 13:17:36 -0400 Subject: [PATCH 5/5] Use InvalidBooleanValueException; return false when encountered --- .../bootstrap/config/provider/ConfigConverter.java | 14 ++++++++++++-- .../bootstrap/config/provider/ConfigProvider.java | 14 ++++++++++++-- .../groovy/datadog/trace/api/ConfigTest.groovy | 2 +- .../config/provider/ConfigConverterTest.groovy | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-) 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 ca9219909c8..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(); /** @@ -422,8 +432,8 @@ public static Boolean booleanValueOf(String value) { } else if ("false".equalsIgnoreCase(value)) { return Boolean.FALSE; } else { - // Throw exception for invalid boolean values - throw new IllegalArgumentException("Invalid boolean value: " + 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 211290056f1..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 (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 IllegalArgumentException + // 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 ef4c13c6211..0a7f1a5e7b3 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -657,7 +657,7 @@ class ConfigTest extends DDSpecification { config.agentHost == " " config.agentPort == 8126 config.agentUrl == "http:// :8126" - config.prioritySamplingEnabled == true + config.prioritySamplingEnabled == false config.traceResolverEnabled == true config.serviceMapping == [:] config.mergedSpanTags == [:] 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 4515f2f5d70..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 @@ -28,7 +28,7 @@ class ConfigConverterTest extends DDSpecification { ConfigConverter.valueOf(invalidValue, Boolean) then: - def exception = thrown(IllegalArgumentException) + def exception = thrown(ConfigConverter.InvalidBooleanValueException) exception.message.contains("Invalid boolean value:") where: