From c8d283ae58331a9c87e00b81af1b53354e5febc3 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:25:38 +0200 Subject: [PATCH 1/7] ReplaceDeprecatedRuntimeExecMethods: preserve the argument vector `Runtime#exec(String)` tokenizes its command with `StringTokenizer`, splitting on ' ', '\t', '\n', '\r' and '\f' and collapsing runs of them. The recipe built the replacement array with `split(" ")` instead, so `exec("printf '%s' value")` became `new String[]{"printf", "", "'%s'", "", "value"}` and no other whitespace was split at all. The rewritten call could launch a process with different arguments than the original. Tokenize literal commands with `StringTokenizer` itself, and leave every other command unchanged, since `command.split(" ")` cannot reproduce the tokenizer at runtime. A command that tokenizes to nothing is also left alone, because `exec("")` throws `IllegalArgumentException` where `exec(new String[]{})` throws `IndexOutOfBoundsException`. Generated tokens are escaped so that quotes, backslashes, control characters and a literal `#{` survive the `JavaTemplate` round trip, and a literal the parser did not decode is declined. Worth weighing: the recipe can no longer modernize non-literal commands, which the description and the `recipes.csv` row now state, and three existing tests that asserted `.split(" ")` output for a variable, a method invocation and a concatenation with a non-constant operand now assert no change. The replacement also copies the parameter type list before overwriting it, because `JavaType.Method` is interned and that list is a write through view shared with every other call of the same overload. --- .../ReplaceDeprecatedRuntimeExecMethods.java | 114 ++++--- .../resources/META-INF/rewrite/recipes.csv | 2 +- ...placeDeprecatedRuntimeExecMethodsTest.java | 317 +++++++++++++++--- 3 files changed, 343 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index d56a1a14f..a706303bb 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -30,6 +30,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.StringTokenizer; public class ReplaceDeprecatedRuntimeExecMethods extends Recipe { private static final MethodMatcher RUNTIME_EXEC_CMD = new MethodMatcher("java.lang.Runtime exec(String)"); @@ -41,7 +42,10 @@ public class ReplaceDeprecatedRuntimeExecMethods extends Recipe { @Getter final String description = "Replace `Runtime#exec(String)` methods to use `exec(String[])` instead because the former is deprecated " + - "after Java 18 and is no longer recommended for use by the Java documentation."; + "after Java 18 and is no longer recommended for use by the Java documentation. Only commands made up entirely of " + + "string literals are replaced, because only then is it known at compile time which arguments " + + "`Runtime#exec(String)` would build; any other command is left unchanged rather than launched with " + + "different arguments."; @Getter final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(3); @@ -71,7 +75,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu StringBuilder sb = new StringBuilder(); if (flattenAble) { for (Expression e : commands) { - if (e instanceof J.Literal && ((J.Literal) e).getType() == JavaType.Primitive.String) { + if (e instanceof J.Literal && ((J.Literal) e).getType() == JavaType.Primitive.String && + isDecoded((J.Literal) e)) { sb.append(((J.Literal) e).getValue()); } else { flattenAble = false; @@ -80,48 +85,38 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } } - updateCursor(m); - if (flattenAble) { - String[] cmds = sb.toString().split(" "); - String templateCode = String.format("new String[] {%s}", toStringArguments(cmds)); - JavaTemplate template = JavaTemplate.builder(templateCode).build(); - - List args = m.getArguments(); - Cursor cursor = new Cursor(getCursor(), args.get(0)); - args.set(0, template.apply(cursor, args.get(0).getCoordinates().replace())); - - if (m.getMethodType() != null) { - List parameterTypes = m.getMethodType().getParameterTypes(); - parameterTypes.set(0, JavaType.ShallowClass.build("java.lang.String[]")); - - return m.withArguments(args) - .withMethodType(m.getMethodType().withParameterTypes(parameterTypes)); - } - } else { - // replace argument to 'command.split(" ")' - List args = m.getArguments(); - boolean needWrap = false; - Expression arg0 = args.get(0); - if (!(arg0 instanceof J.Identifier) && - !(arg0 instanceof J.Literal) && - !(arg0 instanceof J.MethodInvocation)) { - needWrap = true; - } - - String code = needWrap ? "(#{any()}).split(\" \")" : "#{any()}.split(\" \")"; - JavaTemplate template = JavaTemplate.builder(code).contextSensitive().build(); - Cursor cursor = new Cursor(getCursor(), args.get(0)); - arg0 = template.apply(cursor, args.get(0).getCoordinates().replace(), args.get(0)); - args.set(0, arg0); - - if (m.getMethodType() != null) { - List parameterTypes = m.getMethodType().getParameterTypes(); - parameterTypes.set(0, JavaType.ShallowClass.build("java.lang.String[]")); + // Only a command made up entirely of string literals can be converted, because `String#split(" ")` + // does not reproduce `Runtime#exec(String)`, which tokenizes on any of ' ', '\t', '\n', '\r' and + // '\f' and collapses runs of them. Anything else is left alone rather than launching a different + // process. + if (!flattenAble) { + return m; + } - return m.withArguments(args).withMethodType(m.getMethodType().withParameterTypes(parameterTypes)); - } + List cmds = new ArrayList<>(); + for (StringTokenizer tokenizer = new StringTokenizer(sb.toString()); tokenizer.hasMoreTokens(); ) { + cmds.add(tokenizer.nextToken()); + } + // `exec("")` throws `IllegalArgumentException("Empty command")` where `exec(new String[]{})` + // throws `IndexOutOfBoundsException`, so a command that tokenizes to nothing is left alone. + JavaType.Method methodType = m.getMethodType(); + if (cmds.isEmpty() || methodType == null) { return m; } + + updateCursor(m); + JavaTemplate template = JavaTemplate.builder(String.format("new String[] {%s}", toStringArguments(cmds))).build(); + + List args = m.getArguments(); + Cursor cursor = new Cursor(getCursor(), args.get(0)); + args.set(0, template.apply(cursor, args.get(0).getCoordinates().replace())); + + // `getParameterTypes()` is a write through view of the `JavaType.Method`, which is interned and + // therefore shared by every other call of the same overload, so copy it before replacing. + List parameterTypes = new ArrayList<>(methodType.getParameterTypes()); + parameterTypes.set(0, new JavaType.Array(null, JavaType.ShallowClass.build("java.lang.String"), null)); + return m.withArguments(args) + .withMethodType(methodType.withParameterTypes(parameterTypes)); } return m; @@ -129,16 +124,39 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu }); } - private static String toStringArguments(String[] cmds) { + /** + * The source of a string literal is its value plus at least the two quotes around it. Where that does not hold the + * parser did not decode the literal, which happens for a unicode escape of a supplementary character, and the value + * read back is not the command that would be executed. + */ + private static boolean isDecoded(J.Literal literal) { + Object value = literal.getValue(); + String valueSource = literal.getValueSource(); + return value != null && valueSource != null && String.valueOf(value).length() + 2 <= valueSource.length(); + } + + private static String toStringArguments(List cmds) { StringBuilder sb = new StringBuilder(); - for (int i = 0; i < cmds.length; i++) { - String token = cmds[i]; - if (i != 0) { + for (String token : cmds) { + if (sb.length() != 0) { sb.append(", "); } - sb.append("\"") - .append(token) - .append("\""); + sb.append('"'); + for (int i = 0; i < token.length(); i++) { + char c = token.charAt(i); + if (c == '"' || c == '\\') { + sb.append('\\').append(c); + } else if (c < ' ' || (c >= 0x7f && c <= 0x9f) || + (c == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { + // Control characters are escaped so that they stay legible rather than becoming invisible + // bytes. `#{` has to be escaped because this text is also `JavaTemplate` source, where `#{` + // opens a parameter placeholder. + sb.append(String.format("\\u%04x", (int) c)); + } else { + sb.append(c); + } + } + sb.append('"'); } return sb.toString(); } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index bdfbd9e95..5fbb0111b 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -143,7 +143,7 @@ According to the `Collection#toArray(T[])` documentation: However, although it's not intuitive, allocating a right-sized array ahead of time to pass to the API appears to be [generally worse for performance](https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion) according to benchmarking and JVM developers due to a number of implementation details in both Java and the virtual machine. H2 achieved significant performance gains by [switching to empty arrays instead pre-sized ones](https://github.com/h2database/h2database/issues/311).",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, -maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceDeprecatedRuntimeExecMethods,Replace deprecated `Runtime#exec()` methods,Replace `Runtime#exec(String)` methods to use `exec(String[])` instead because the former is deprecated after Java 18 and is no longer recommended for use by the Java documentation.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceDeprecatedRuntimeExecMethods,Replace deprecated `Runtime#exec()` methods,"Replace `Runtime#exec(String)` methods to use `exec(String[])` instead because the former is deprecated after Java 18 and is no longer recommended for use by the Java documentation. Only commands made up entirely of string literals are replaced, because only then is it known at compile time which arguments `Runtime#exec(String)` would build; any other command is left unchanged rather than launched with different arguments.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceDuplicateStringLiterals,Replace duplicate `String` literals,"Replaces `String` literals with a length of 5 or greater repeated a minimum of 3 times. Qualified `String` literals include final Strings, method invocations, and new class invocations. Adds a new `private static final String` or uses an existing equivalent class field. A new variable name will be generated based on the literal value if an existing field does not exist. The generated name will append a numeric value to the variable name if a name already exists in the compilation unit. Centralizing repeated string values into constants makes refactoring safer and reduces the risk of inconsistent updates.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""includeTestSources"",""type"":""Boolean"",""displayName"":""Apply recipe to test source set"",""description"":""Changes only apply to main by default. `includeTestSources` will apply the recipe to `test` source files.""}]", maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceHashtableWithHashMap,Replace `java.util.Hashtable` with `java.util.HashMap`,"`Hashtable` synchronizes every operation, which adds overhead in the common single-threaded case. This recipe replaces a local `Hashtable` with a `HashMap` when data flow analysis can prove the `Hashtable` never escapes its method (it is not returned, assigned to a field, or passed as an argument), so no other thread can observe it and the synchronization is redundant. Fields, escaping variables, and `Hashtable`-specific method usages (`contains`, `elements`, `keys`) are left untouched. `HashMap` permits `null` keys and values, so it accepts every input `Hashtable` did.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated"",""displayName"":""Legacy synchronized types not migrated"",""instanceName"":""Legacy synchronized types not migrated"",""description"":""Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that were found but left unchanged because they could not be proven safe to modernize."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the unmigrated reference.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the reference.""},{""name"":""unmigratedType"",""type"":""String"",""displayName"":""Unmigrated type"",""description"":""The fully qualified name of the legacy synchronized type that was found but not migrated.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the instance was left unchanged.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference,Use method references in lambda,"Replaces the single statement lambdas `o -> o instanceOf X`, `o -> (A) o`, `o -> System.out.println(o)`, `o -> o != null`, `o -> o == null` with the equivalent method reference. Method references are often more concise and readable than their lambda equivalents, making the code's intent clearer at a glance.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java index 18de6d4ef..535537441 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java @@ -73,7 +73,7 @@ void method() throws IOException { } @Test - void stringVariableAsInput() { + void repeatedDelimitersInRawString() { rewriteRun( version( //language=java @@ -82,15 +82,213 @@ void stringVariableAsInput() { import java.io.File; import java.io.IOException; + class A { + void method(Runtime runtime, String[] envp, File dir) throws IOException { + runtime.exec("printf '%s' value"); + runtime.exec("printf '%s' value", envp); + runtime.exec("printf '%s' value", envp, dir); + } + } + """, + """ + import java.io.File; + import java.io.IOException; + + class A { + void method(Runtime runtime, String[] envp, File dir) throws IOException { + runtime.exec(new String[]{"printf", "'%s'", "value"}); + runtime.exec(new String[]{"printf", "'%s'", "value"}, envp); + runtime.exec(new String[]{"printf", "'%s'", "value"}, envp, dir); + } + } + """ + ), 18) + ); + } + + @Test + void allTokenizerDelimitersInRawString() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(" \\tls\\n-a \\r\\r -l\\f-h "); + } + } + """, + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"ls", "-a", "-l", "-h"}); + } + } + """ + ), 18) + ); + } + + @Test + void quotesAndBackslashesInRawString() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec("echo \\"a b\\" C:\\\\dir"); + } + } + """, + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"echo", "\\"a", "b\\"", "C:\\\\dir"}); + } + } + """ + ), 18) + ); + } + + @Test + void controlCharactersInRawString() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec("echo a\\u000bb \\u007f"); + } + } + """, + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"echo", "a\\u000bb", "\\u007f"}); + } + } + """ + ), 18) + ); + } + + @Test + void templatePlaceholderInRawString() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec("sed -e s/#{a}/b/ f.txt"); + } + } + """, + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"sed", "-e", "s/\\u0023{a}/b/", "f.txt"}); + } + } + """ + ), 18) + ); + } + + @Test + void everyCallOfTheSameOverloadIsReplaced() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec("ls -a"); + runtime.exec("ps -e"); + } + } + """, + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"ls", "-a"}); + runtime.exec(new String[]{"ps", "-e"}); + } + } + """ + ), 18), + version( + //language=java + java( + """ + import java.io.IOException; + class B { - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); - String command = "ls -al"; - String[] envp = { "E1=1", "E2=2"}; - File dir = new File("/tmp"); - Process process1 = runtime.exec(command); - Process process2 = runtime.exec(command, envp); - Process process3 = runtime.exec(command, envp, dir); + void method(Runtime runtime) throws IOException { + runtime.exec("df -h"); + } + } + """, + """ + import java.io.IOException; + + class B { + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"df", "-h"}); + } + } + """ + ), 18) + ); + } + + @Test + void rawStringWithSideEffectingEnvironmentAndDirectory() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.File; + import java.io.IOException; + + class A { + String[] envp() { + return new String[]{"E1=1"}; + } + File dir() { + return new File("/tmp"); + } + void method(Runtime runtime) throws IOException { + runtime.exec("ls -a", envp(), dir()); } } """, @@ -98,15 +296,37 @@ void method() throws IOException { import java.io.File; import java.io.IOException; + class A { + String[] envp() { + return new String[]{"E1=1"}; + } + File dir() { + return new File("/tmp"); + } + void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"ls", "-a"}, envp(), dir()); + } + } + """ + ), 18) + ); + } + + @Test + void doNotChangeStringVariableAsInput() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.File; + import java.io.IOException; + class B { - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); - String command = "ls -al"; - String[] envp = { "E1=1", "E2=2"}; - File dir = new File("/tmp"); - Process process1 = runtime.exec(command.split(" ")); - Process process2 = runtime.exec(command.split(" "), envp); - Process process3 = runtime.exec(command.split(" "), envp, dir); + void method(Runtime runtime, String command, String[] envp, File dir) throws IOException { + Process process1 = runtime.exec(command); + Process process2 = runtime.exec(command, envp); + Process process3 = runtime.exec(command, envp, dir); } } """ @@ -115,7 +335,7 @@ void method() throws IOException { } @Test - void methodInvocationAsInput() { + void doNotChangeMethodInvocationAsInput() { rewriteRun( version( //language=java @@ -127,22 +347,49 @@ class B { String command() { return "ls -al"; } - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); + void method(Runtime runtime) throws IOException { Process process = runtime.exec(command()); } } - """, + """ + ), 18) + ); + } + + @Test + void doNotChangeCommandsThatFailAtRuntime() { + rewriteRun( + version( + //language=java + java( """ import java.io.IOException; - class B { - String command() { - return "ls -al"; + class A { + void method(Runtime runtime) throws IOException { + runtime.exec(""); + runtime.exec(" "); + runtime.exec("\\t\\n\\r\\f"); + runtime.exec((String) null); } - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); - Process process = runtime.exec(command().split(" ")); + } + """ + ), 18) + ); + } + + @Test + void doNotChangeCommandWithSupplementaryCharacterEscape() { + rewriteRun( + version( + //language=java + java( + """ + import java.io.IOException; + + class A { + void method(Runtime runtime) throws IOException { + runtime.exec("echo \\ud83d\\ude00x"); } } """ @@ -181,7 +428,7 @@ void method() throws IOException { } @Test - void concatenatedObjectsAsInput() { + void doNotChangeConcatenatedObjectsAsInput() { rewriteRun( version( //language=java @@ -191,22 +438,10 @@ void concatenatedObjectsAsInput() { class B { String options = "-a -l"; - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); + void method(Runtime runtime) throws IOException { Process process = runtime.exec("ls" + " " + options); } } - """, - """ - import java.io.IOException; - - class B { - String options = "-a -l"; - void method() throws IOException { - Runtime runtime = Runtime.getRuntime(); - Process process = runtime.exec(("ls" + " " + options).split(" ")); - } - } """ ), 18) ); From 0d94141f040427cde9bc0d2d87ca000dcc45c155 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 11:30:18 +0200 Subject: [PATCH 2/7] Review fixes: detect undecoded literals through getUnicodeEscapes() --- .../ReplaceDeprecatedRuntimeExecMethods.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index a706303bb..ef55a6255 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -85,10 +85,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } } - // Only a command made up entirely of string literals can be converted, because `String#split(" ")` - // does not reproduce `Runtime#exec(String)`, which tokenizes on any of ' ', '\t', '\n', '\r' and - // '\f' and collapses runs of them. Anything else is left alone rather than launching a different - // process. + // Only a command made up entirely of string literals can be tokenized the way + // `Runtime#exec(String)` does, on any of ' ', '\t', '\n', '\r' and '\f', collapsing runs of them. + // Anything else is left alone rather than launching a different process. if (!flattenAble) { return m; } @@ -125,14 +124,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } /** - * The source of a string literal is its value plus at least the two quotes around it. Where that does not hold the - * parser did not decode the literal, which happens for a unicode escape of a supplementary character, and the value - * read back is not the command that would be executed. + * When the parser records unicode escapes it stores the undecoded source in the value as well, so the value read + * back is not the command that would be executed. */ private static boolean isDecoded(J.Literal literal) { - Object value = literal.getValue(); - String valueSource = literal.getValueSource(); - return value != null && valueSource != null && String.valueOf(value).length() + 2 <= valueSource.length(); + List unicodeEscapes = literal.getUnicodeEscapes(); + return literal.getValue() != null && (unicodeEscapes == null || unicodeEscapes.isEmpty()); } private static String toStringArguments(List cmds) { From 7941775f5caa957bc106d1f61b5588efd2ea2108 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:23:19 +0200 Subject: [PATCH 3/7] Trim commentary --- .../ReplaceDeprecatedRuntimeExecMethods.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index ef55a6255..d74353633 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -85,9 +85,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } } - // Only a command made up entirely of string literals can be tokenized the way - // `Runtime#exec(String)` does, on any of ' ', '\t', '\n', '\r' and '\f', collapsing runs of them. - // Anything else is left alone rather than launching a different process. + // Only an all-literal command can be tokenized the way `Runtime#exec(String)` does, on any of + // ' ', '\t', '\n', '\r' and '\f', collapsing runs; anything else would launch a different process if (!flattenAble) { return m; } @@ -96,8 +95,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu for (StringTokenizer tokenizer = new StringTokenizer(sb.toString()); tokenizer.hasMoreTokens(); ) { cmds.add(tokenizer.nextToken()); } - // `exec("")` throws `IllegalArgumentException("Empty command")` where `exec(new String[]{})` - // throws `IndexOutOfBoundsException`, so a command that tokenizes to nothing is left alone. + // `exec("")` throws `IllegalArgumentException` where `exec(new String[]{})` throws + // `IndexOutOfBoundsException`, so a command tokenizing to nothing is left alone JavaType.Method methodType = m.getMethodType(); if (cmds.isEmpty() || methodType == null) { return m; @@ -110,8 +109,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu Cursor cursor = new Cursor(getCursor(), args.get(0)); args.set(0, template.apply(cursor, args.get(0).getCoordinates().replace())); - // `getParameterTypes()` is a write through view of the `JavaType.Method`, which is interned and - // therefore shared by every other call of the same overload, so copy it before replacing. + // `getParameterTypes()` writes through to the interned `JavaType.Method` shared by every call + // of this overload, so copy before replacing List parameterTypes = new ArrayList<>(methodType.getParameterTypes()); parameterTypes.set(0, new JavaType.Array(null, JavaType.ShallowClass.build("java.lang.String"), null)); return m.withArguments(args) @@ -145,9 +144,8 @@ private static String toStringArguments(List cmds) { sb.append('\\').append(c); } else if (c < ' ' || (c >= 0x7f && c <= 0x9f) || (c == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { - // Control characters are escaped so that they stay legible rather than becoming invisible - // bytes. `#{` has to be escaped because this text is also `JavaTemplate` source, where `#{` - // opens a parameter placeholder. + // Control characters stay legible rather than becoming invisible bytes, and `#{` needs escaping + // because this text is also `JavaTemplate` source sb.append(String.format("\\u%04x", (int) c)); } else { sb.append(c); From 8e69acc8de67416f73a97985779ce4847e6287c7 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 15:16:18 +0200 Subject: [PATCH 4/7] Consolidate Runtime.exec tokenization tests --- .../ReplaceDeprecatedRuntimeExecMethods.java | 19 ++-- ...placeDeprecatedRuntimeExecMethodsTest.java | 86 +------------------ 2 files changed, 12 insertions(+), 93 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index d74353633..f13bc73ff 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -122,10 +122,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu }); } - /** - * When the parser records unicode escapes it stores the undecoded source in the value as well, so the value read - * back is not the command that would be executed. - */ + // getValue() is not fully decoded when the literal contains Unicode escapes. private static boolean isDecoded(J.Literal literal) { List unicodeEscapes = literal.getUnicodeEscapes(); return literal.getValue() != null && (unicodeEscapes == null || unicodeEscapes.isEmpty()); @@ -139,16 +136,16 @@ private static String toStringArguments(List cmds) { } sb.append('"'); for (int i = 0; i < token.length(); i++) { - char c = token.charAt(i); - if (c == '"' || c == '\\') { - sb.append('\\').append(c); - } else if (c < ' ' || (c >= 0x7f && c <= 0x9f) || - (c == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { + char character = token.charAt(i); + if (character == '"' || character == '\\') { + sb.append('\\').append(character); + } else if (character < ' ' || (character >= 0x7f && character <= 0x9f) || + (character == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { // Control characters stay legible rather than becoming invisible bytes, and `#{` needs escaping // because this text is also `JavaTemplate` source - sb.append(String.format("\\u%04x", (int) c)); + sb.append(String.format("\\u%04x", (int) character)); } else { - sb.append(c); + sb.append(character); } } sb.append('"'); diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java index 535537441..8c0ad6e46 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethodsTest.java @@ -107,7 +107,7 @@ void method(Runtime runtime, String[] envp, File dir) throws IOException { } @Test - void allTokenizerDelimitersInRawString() { + void tokenizeRawStrings() { rewriteRun( version( //language=java @@ -118,89 +118,8 @@ void allTokenizerDelimitersInRawString() { class A { void method(Runtime runtime) throws IOException { runtime.exec(" \\tls\\n-a \\r\\r -l\\f-h "); - } - } - """, - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { - runtime.exec(new String[]{"ls", "-a", "-l", "-h"}); - } - } - """ - ), 18) - ); - } - - @Test - void quotesAndBackslashesInRawString() { - rewriteRun( - version( - //language=java - java( - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { runtime.exec("echo \\"a b\\" C:\\\\dir"); - } - } - """, - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { - runtime.exec(new String[]{"echo", "\\"a", "b\\"", "C:\\\\dir"}); - } - } - """ - ), 18) - ); - } - - @Test - void controlCharactersInRawString() { - rewriteRun( - version( - //language=java - java( - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { runtime.exec("echo a\\u000bb \\u007f"); - } - } - """, - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { - runtime.exec(new String[]{"echo", "a\\u000bb", "\\u007f"}); - } - } - """ - ), 18) - ); - } - - @Test - void templatePlaceholderInRawString() { - rewriteRun( - version( - //language=java - java( - """ - import java.io.IOException; - - class A { - void method(Runtime runtime) throws IOException { runtime.exec("sed -e s/#{a}/b/ f.txt"); } } @@ -210,6 +129,9 @@ void method(Runtime runtime) throws IOException { class A { void method(Runtime runtime) throws IOException { + runtime.exec(new String[]{"ls", "-a", "-l", "-h"}); + runtime.exec(new String[]{"echo", "\\"a", "b\\"", "C:\\\\dir"}); + runtime.exec(new String[]{"echo", "a\\u000bb", "\\u007f"}); runtime.exec(new String[]{"sed", "-e", "s/\\u0023{a}/b/", "f.txt"}); } } From 3a897b7b283511384ea71c96179a037f3cd11eaa Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:04:50 +0200 Subject: [PATCH 5/7] Replace control-character ranges with Character.isISOControl --- .../staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index f13bc73ff..ab72ed065 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -139,7 +139,7 @@ private static String toStringArguments(List cmds) { char character = token.charAt(i); if (character == '"' || character == '\\') { sb.append('\\').append(character); - } else if (character < ' ' || (character >= 0x7f && character <= 0x9f) || + } else if (Character.isISOControl(character) || (character == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { // Control characters stay legible rather than becoming invisible bytes, and `#{` needs escaping // because this text is also `JavaTemplate` source From 60064baea79036ee0a63dfcae4612ff5f215f256 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:35:29 +0200 Subject: [PATCH 6/7] Extract unicodeEscape explaining method for the control-character format string --- .../ReplaceDeprecatedRuntimeExecMethods.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index ab72ed065..33575bb00 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -143,7 +143,7 @@ private static String toStringArguments(List cmds) { (character == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { // Control characters stay legible rather than becoming invisible bytes, and `#{` needs escaping // because this text is also `JavaTemplate` source - sb.append(String.format("\\u%04x", (int) character)); + sb.append(unicodeEscape(character)); } else { sb.append(character); } @@ -152,4 +152,9 @@ private static String toStringArguments(List cmds) { } return sb.toString(); } + + // Formats the character as a Java Unicode escape, `\u0007` for the bell character + private static String unicodeEscape(char character) { + return String.format("\\u%04x", (int) character); + } } From cc263e81bfcd79be22556939e018c40c469893b8 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:42:25 +0200 Subject: [PATCH 7/7] Name the template interpolation marker and explain the escape prefix --- .../staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java index 33575bb00..1a1fb7101 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDeprecatedRuntimeExecMethods.java @@ -128,6 +128,8 @@ private static boolean isDecoded(J.Literal literal) { return literal.getValue() != null && (unicodeEscapes == null || unicodeEscapes.isEmpty()); } + private static final String TEMPLATE_INTERPOLATION_START = "#{"; + private static String toStringArguments(List cmds) { StringBuilder sb = new StringBuilder(); for (String token : cmds) { @@ -138,9 +140,10 @@ private static String toStringArguments(List cmds) { for (int i = 0; i < token.length(); i++) { char character = token.charAt(i); if (character == '"' || character == '\\') { + // Prefix a backslash so the character survives inside the generated string literal sb.append('\\').append(character); } else if (Character.isISOControl(character) || - (character == '#' && i + 1 < token.length() && token.charAt(i + 1) == '{')) { + token.startsWith(TEMPLATE_INTERPOLATION_START, i)) { // Control characters stay legible rather than becoming invisible bytes, and `#{` needs escaping // because this text is also `JavaTemplate` source sb.append(unicodeEscape(character));