Skip to content

Commit dd6e5d6

Browse files
mtoffl01mcculls
andauthored
Throw InvalidBooleanValueException in ConfigConverter.booleanValueOf (#9370)
* throw IllegalArgumentException in ConfigConverter.booleanValueOf * modify get to only catch IllegalArgumentException * Fix failing ConfigTest tests * Update dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com> * Use InvalidBooleanValueException; return false when encountered --------- Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com>
1 parent c2e8852 commit dd6e5d6

File tree

4 files changed

+76
-13
lines changed

4 files changed

+76
-13
lines changed

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ final class ConfigConverter {
2121

2222
private static final Logger log = LoggerFactory.getLogger(ConfigConverter.class);
2323

24+
/**
25+
* Custom exception for invalid boolean values to maintain backward compatibility. When caught in
26+
* ConfigProvider, boolean methods should return false instead of default value.
27+
*/
28+
static class InvalidBooleanValueException extends IllegalArgumentException {
29+
public InvalidBooleanValueException(String message) {
30+
super(message);
31+
}
32+
}
33+
2434
private static final ValueOfLookup LOOKUP = new ValueOfLookup();
2535

2636
/**
@@ -39,6 +49,8 @@ static <T> T valueOf(final String value, @Nonnull final Class<T> tClass) {
3949
return (T) LOOKUP.get(tClass).invoke(value);
4050
} catch (final NumberFormatException e) {
4151
throw e;
52+
} catch (final IllegalArgumentException e) {
53+
throw e;
4254
} catch (final Throwable e) {
4355
log.debug("Can't parse: ", e);
4456
throw new NumberFormatException(e.toString());
@@ -413,8 +425,15 @@ static BitSet parseIntegerRangeSet(@Nonnull String str, final String settingName
413425
public static Boolean booleanValueOf(String value) {
414426
if ("1".equals(value)) {
415427
return Boolean.TRUE;
428+
} else if ("0".equals(value)) {
429+
return Boolean.FALSE;
430+
} else if ("true".equalsIgnoreCase(value)) {
431+
return Boolean.TRUE;
432+
} else if ("false".equalsIgnoreCase(value)) {
433+
return Boolean.FALSE;
416434
} else {
417-
return Boolean.valueOf(value);
435+
// Throw custom exception for invalid boolean values to maintain backward compatibility
436+
throw new InvalidBooleanValueException("Invalid boolean value: " + value);
418437
}
419438
}
420439

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,17 +193,27 @@ public double getDouble(String key, double defaultValue) {
193193

194194
private <T> T get(String key, T defaultValue, Class<T> type, String... aliases) {
195195
for (ConfigProvider.Source source : sources) {
196+
String sourceValue = source.get(key, aliases);
196197
try {
197-
String sourceValue = source.get(key, aliases);
198198
T value = ConfigConverter.valueOf(sourceValue, type);
199199
if (value != null) {
200200
if (collectConfig) {
201201
ConfigCollector.get().put(key, sourceValue, source.origin());
202202
}
203203
return value;
204204
}
205-
} catch (NumberFormatException ex) {
206-
// continue
205+
} catch (ConfigConverter.InvalidBooleanValueException ex) {
206+
// For backward compatibility: invalid boolean values should return false, not default
207+
// Store the invalid sourceValue for telemetry, but return false for the application
208+
if (Boolean.class.equals(type)) {
209+
if (collectConfig) {
210+
ConfigCollector.get().put(key, sourceValue, source.origin());
211+
}
212+
return (T) Boolean.FALSE;
213+
}
214+
// For non-boolean types, continue to next source
215+
} catch (IllegalArgumentException ex) {
216+
// continue - covers both NumberFormatException and other IllegalArgumentException
207217
}
208218
}
209219
if (collectConfig) {

internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,17 +2042,12 @@ class ConfigTest extends DDSpecification {
20422042
where:
20432043
// spotless:off
20442044
value | tClass | expected
2045-
"42.42" | Boolean | false
2046-
"42.42" | Boolean | false
20472045
"true" | Boolean | true
20482046
"trUe" | Boolean | true
2049-
"trUe" | Boolean | true
2050-
"tru" | Boolean | false
2051-
"truee" | Boolean | false
2052-
"true " | Boolean | false
2053-
" true" | Boolean | false
2054-
" true " | Boolean | false
2055-
" true " | Boolean | false
2047+
"false" | Boolean | false
2048+
"False" | Boolean | false
2049+
"1" | Boolean | true
2050+
"0" | Boolean | false
20562051
"42.42" | Float | 42.42f
20572052
"42.42" | Double | 42.42
20582053
"44" | Integer | 44
@@ -2079,6 +2074,20 @@ class ConfigTest extends DDSpecification {
20792074
// spotless:on
20802075
}
20812076

2077+
def "valueOf negative test for invalid boolean values"() {
2078+
when:
2079+
ConfigConverter.valueOf(value, Boolean)
2080+
2081+
then:
2082+
def exception = thrown(IllegalArgumentException)
2083+
exception.message.contains("Invalid boolean value")
2084+
2085+
where:
2086+
// spotless:off
2087+
value << ["42.42", "tru", "truee", "true ", " true", " true ", " true ", "notABool", "invalid", "yes", "no", "42"]
2088+
// spotless:on
2089+
}
2090+
20822091
def "valueOf negative test"() {
20832092
when:
20842093
ConfigConverter.valueOf(value, tClass)

internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,31 @@ class ConfigConverterTest extends DDSpecification {
2323
"0" | false
2424
}
2525

26+
def "Convert boolean properties throws exception for invalid values"() {
27+
when:
28+
ConfigConverter.valueOf(invalidValue, Boolean)
29+
30+
then:
31+
def exception = thrown(ConfigConverter.InvalidBooleanValueException)
32+
exception.message.contains("Invalid boolean value:")
33+
34+
where:
35+
invalidValue << [
36+
"42.42",
37+
"tru",
38+
"truee",
39+
"true ",
40+
" true",
41+
" true ",
42+
" true ",
43+
"notABool",
44+
"yes",
45+
"no",
46+
"on",
47+
"off"
48+
]
49+
}
50+
2651
def "parse map properly for #mapString"() {
2752
when:
2853
def result = ConfigConverter.parseMap(mapString, "test")

0 commit comments

Comments
 (0)