diff --git a/common/network-common/src/test/java/org/apache/spark/network/util/JavaUtilsSuite.java b/common/network-common/src/test/java/org/apache/spark/network/util/JavaUtilsSuite.java index 64edc0edae6f3..78a68a6622546 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/util/JavaUtilsSuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/util/JavaUtilsSuite.java @@ -90,4 +90,25 @@ public void testListFiles() throws IOException { assertEquals(1, JavaUtils.listFiles(symlink).size()); assertEquals(1, JavaUtils.listPaths(symlink).size()); } + + @Test + public void testCheckArgumentUsesMessageVerbatim() { + // The no-varargs overload must not run the message through String.format: a '%' in the + // message is preserved rather than interpreted as a (here invalid) format conversion, which + // would otherwise throw an IllegalFormatException and mask the intended error. + String msg = "invalid value: 100% done, %d unparsed"; + assertDoesNotThrow(() -> JavaUtils.checkArgument(true, msg)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> JavaUtils.checkArgument(false, msg)); + assertEquals(msg, e.getMessage()); + } + + @Test + public void testCheckStateUsesMessageVerbatim() { + String msg = "bad state: 50% done, %s missing"; + assertDoesNotThrow(() -> JavaUtils.checkState(true, msg)); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> JavaUtils.checkState(false, msg)); + assertEquals(msg, e.getMessage()); + } } diff --git a/common/utils-java/src/main/java/org/apache/spark/network/util/JavaUtils.java b/common/utils-java/src/main/java/org/apache/spark/network/util/JavaUtils.java index 2cf4570488ee0..acd9d188a9399 100644 --- a/common/utils-java/src/main/java/org/apache/spark/network/util/JavaUtils.java +++ b/common/utils-java/src/main/java/org/apache/spark/network/util/JavaUtils.java @@ -760,6 +760,29 @@ public static void checkArgument(boolean check, String msg, Object... args) { } } + /** + * Throws an {@link IllegalArgumentException} with the given message if {@code check} is + * {@code false}. + * + *
Unlike {@link #checkArgument(boolean, String, Object...)}, {@code msg} is used verbatim and + * is not passed through {@link String#format(String, Object...)}. Prefer this overload + * whenever the message is a fixed string or is assembled by the caller (for example by + * concatenation): the message may then contain {@code %} characters without risking a + * {@link java.util.IllegalFormatException} that would mask the intended error, and no format + * arguments array is allocated. Use the varargs overload only when there are values to + * interpolate, passing them as arguments (e.g. {@code checkArgument(cond, "bad key: %s", key)}) + * rather than concatenating them into {@code msg}. + * + * @param check the condition that must hold; an exception is thrown when it is {@code false} + * @param msg the failure message, used exactly as given + * @throws IllegalArgumentException if {@code check} is {@code false} + */ + public static void checkArgument(boolean check, String msg) { + if (!check) { + throw new IllegalArgumentException(msg); + } + } + /** * Throws IllegalStateException with the given message if the check is false. * Keep this clone of CommandBuilderUtils.checkState synced with the original. @@ -770,6 +793,24 @@ public static void checkState(boolean check, String msg, Object... args) { } } + /** + * Throws an {@link IllegalStateException} with the given message if {@code check} is + * {@code false}. + * + *
Unlike {@link #checkState(boolean, String, Object...)}, {@code msg} is used verbatim and is + * not passed through {@link String#format(String, Object...)}; see + * {@link #checkArgument(boolean, String)} for when to prefer this form over the varargs one. + * + * @param check the condition that must hold; an exception is thrown when it is {@code false} + * @param msg the failure message, used exactly as given + * @throws IllegalStateException if {@code check} is {@code false} + */ + public static void checkState(boolean check, String msg) { + if (!check) { + throw new IllegalStateException(msg); + } + } + private static final HexFormat LOWERCASE_HEX = HexFormat.of(); /**