Skip to content

Commit

Permalink
Make String to Boolean parameter conversion more strict (#3178)
Browse files Browse the repository at this point in the history
Cause use of anything other than `true` or `false` (case-insensitive)
fail in conversion, to make potential typos explicitly visible (instead
of implicitly just treating all else as `false`).

Includes a few tests for failures on invalid values, for conversions
handled by `StringToBooleanAndCharPrimitiveConverter`.

Resolves #3177.

Co-authored-by: Marc Philipp <mail@marcphilipp.de>
  • Loading branch information
s-rwe and marcphilipp committed Apr 28, 2023
1 parent 75fbe27 commit f79e38b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ repository on GitHub.

* The `dynamic` parallel execution strategy now allows the thread pool to be saturated by
default.
* Implicit type conversion of boolean values like in `@CsvSource` is now stricter, only
allowing values `"true"` or `"false"` (case-insensitive), in order to make accidental
mistakes apparent, avoiding potential confusion.

==== New Features and Improvements

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,7 @@ integral types: `byte`, `short`, `int`, `long`, and their boxed counterparts.
|===
| Target Type | Example

| `boolean`/`Boolean` | `"true"` -> `true`
| `boolean`/`Boolean` | `"true"` -> `true` _(only accepts values 'true' or 'false', case-insensitive)_
| `byte`/`Byte` | `"15"`, `"0xF"`, or `"017"` -> `(byte) 15`
| `char`/`Character` | `"o"` -> `'o'`
| `short`/`Short` | `"15"`, `"0xF"`, or `"017"` -> `(short) 15`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,12 @@ private static class StringToBooleanAndCharPrimitiveConverter implements StringT
private static final Map<Class<?>, Function<String, ?>> CONVERTERS;
static {
Map<Class<?>, Function<String, ?>> converters = new HashMap<>();
converters.put(Boolean.class, Boolean::valueOf);
converters.put(Boolean.class, source -> {
boolean isTrue = "true".equalsIgnoreCase(source);
Preconditions.condition(isTrue || "false".equalsIgnoreCase(source),
() -> "String must be (ignoring case) 'true' or 'false': " + source);
return isTrue;
});
converters.put(Character.class, source -> {
Preconditions.condition(source.length() == 1, () -> "String must have length of 1: " + source);
return source.charAt(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.junit.jupiter.params.converter;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

import java.io.File;
import java.lang.Thread.State;
Expand Down Expand Up @@ -99,6 +100,21 @@ void convertsStringsToPrimitiveTypes() {
assertConverts("42.2_3", double.class, 42.23);
}

@Test
void throwsExceptionOnInvalidStringForPrimitiveTypes() {
assertThatExceptionOfType(ArgumentConversionException.class) //
.isThrownBy(() -> convert("ab", char.class)) //
.withMessage("Failed to convert String \"ab\" to type java.lang.Character") //
.havingCause() //
.withMessage("String must have length of 1: ab");

assertThatExceptionOfType(ArgumentConversionException.class) //
.isThrownBy(() -> convert("tru", boolean.class)) //
.withMessage("Failed to convert String \"tru\" to type java.lang.Boolean") //
.havingCause() //
.withMessage("String must be (ignoring case) 'true' or 'false': tru");
}

/**
* @since 5.4
*/
Expand Down Expand Up @@ -232,11 +248,14 @@ void convertsStringToUUID() {
// -------------------------------------------------------------------------

private void assertConverts(Object input, Class<?> targetClass, Object expectedOutput) {
var result = DefaultArgumentConverter.INSTANCE.convert(input, targetClass);
var result = convert(input, targetClass);

assertThat(result) //
.describedAs(input + " --(" + targetClass.getName() + ")--> " + expectedOutput) //
.isEqualTo(expectedOutput);
}

private Object convert(Object input, Class<?> targetClass) {
return DefaultArgumentConverter.INSTANCE.convert(input, targetClass);
}
}

0 comments on commit f79e38b

Please sign in to comment.