From 87f55644f75ce9779e82655395e8163d958925ee Mon Sep 17 00:00:00 2001 From: Mingye Wang Date: Sun, 5 Jun 2022 12:15:23 +0800 Subject: [PATCH 1/9] CommandBuilder: Use more robust quoting * A while ago I brewed a pretty good quoting thing with unit tests on CMD (https://github.com/Artoria2e5/node/blob/fix!/child-process-args/test/parallel/test-child-process-spawnsync-shell-args.js). Use that. Specifically, the batch language and MSVCRT interprets the whole quoted thing a bit differently and we need to quote for both. An example that I know breaks is `He\\\\o \\"Wor\tld\\|<>&*\\`. * The shell quote is overcomplicated. The correct way is a lot like PWSH, in that we have a simple single quote. --- .../hmcl/util/platform/CommandBuilder.java | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index bff944c913..b93951cac9 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -45,16 +45,16 @@ public CommandBuilder(OperatingSystem os) { this.os = os; } - private String parse(String s) { + private String quote(String s) { if (OperatingSystem.WINDOWS == os) { - return parseBatch(s); + return quoteBatch(s); } else { - return parseShell(s); + return quoteShell(s); } } /** - * Parsing will ignore your manual escaping + * "Parsing" (quoting) will ignore your manual escaping * * @param args commands * @return this @@ -142,7 +142,7 @@ public boolean removeIf(Predicate pred) { @Override public String toString() { - return raw.stream().map(i -> i.parse ? parse(i.arg) : i.arg).collect(Collectors.joining(" ")); + return raw.stream().map(i -> i.quote ? quote(i.arg) : i.arg).collect(Collectors.joining(" ")); } public List asList() { @@ -155,19 +155,20 @@ public List asMutableList() { private static class Item { String arg; - boolean parse; + boolean quote; - Item(String arg, boolean parse) { + Item(String arg, boolean quote) { this.arg = arg; - this.parse = parse; + this.quote = quote; } @Override public String toString() { - return parse ? (OperatingSystem.WINDOWS == OperatingSystem.CURRENT_OS ? parseBatch(arg) : parseShell(arg)) : arg; + return quote ? (OperatingSystem.WINDOWS == OperatingSystem.CURRENT_OS ? quoteBatch(arg) : quoteShell(arg)) : arg; } } + // Quote for powershell. public static String pwshString(String str) { return "'" + str.replace("'", "''") + "'"; } @@ -206,28 +207,24 @@ public static boolean setExecutionPolicy() { return true; } - private static String parseBatch(String s) { - String escape = " \t\"^&<>|"; + private static String quoteBatch(String s) { + String escape = " \t\"^&<>|?*"; if (StringUtils.containsOne(s, escape.toCharArray())) // The argument has not been quoted, add quotes. - return '"' + s - .replace("\\", "\\\\") - .replace("\"", "\\\"") - + '"'; + // See explanation at https://github.com/Artoria2e5/node/blob/fix!/child-process-args/lib/child_process.js + // about making the string "inert to CMD", and associated unit tests + return '"' + s.replaceAll('(\\*)($|")', '$1$1$2') + '"'; else { return s; } } - private static String parseShell(String s) { - String escaping = " \t\"!#$&'()*,;<=>?[\\]^`{|}~"; - String escaped = "\"$&`"; - if (s.indexOf(' ') >= 0 || s.indexOf('\t') >= 0 || StringUtils.containsOne(s, escaping.toCharArray())) { - // The argument has not been quoted, add quotes. - for (char ch : escaped.toCharArray()) - s = s.replace("" + ch, "\\" + ch); - return '"' + s + '"'; - } else + private static String quoteShell(String s) { + String escape = " \t\"!#$&'()*,;<=>?[\\]^`{|}~"; + if (StringUtils.containsOne(s, escape.toCharArray())) { + return "'" + s.replace("'", "'\''") + "'"; + } else { return s; + } } } From 25e91133c217c3f91c03259c38a879c22aca1cd9 Mon Sep 17 00:00:00 2001 From: Mingye Wang Date: Sun, 5 Jun 2022 12:19:56 +0800 Subject: [PATCH 2/9] fixup! so it's more like C... (SQUASH MERGE!) --- .../java/org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index b93951cac9..394bf673c4 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -213,7 +213,7 @@ private static String quoteBatch(String s) { // The argument has not been quoted, add quotes. // See explanation at https://github.com/Artoria2e5/node/blob/fix!/child-process-args/lib/child_process.js // about making the string "inert to CMD", and associated unit tests - return '"' + s.replaceAll('(\\*)($|")', '$1$1$2') + '"'; + return '"' + s.replaceAll("(\\\\*)($|")"", "$1$1$2") + '"'; else { return s; } From b031549430444786faed01041155574336272636 Mon Sep 17 00:00:00 2001 From: Mingye Wang Date: Sun, 5 Jun 2022 12:24:29 +0800 Subject: [PATCH 3/9] fixup! missed a step! --- .../java/org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index 394bf673c4..ce7d507c0c 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -213,7 +213,7 @@ private static String quoteBatch(String s) { // The argument has not been quoted, add quotes. // See explanation at https://github.com/Artoria2e5/node/blob/fix!/child-process-args/lib/child_process.js // about making the string "inert to CMD", and associated unit tests - return '"' + s.replaceAll("(\\\\*)($|")"", "$1$1$2") + '"'; + return '"' + s.replaceAll("(\\\\*)($|")"", "$1$1$2").replace("\"", "\"\"") + '"'; else { return s; } From 9c610b6458335b7ebb0665cd09a63a5192808dcb Mon Sep 17 00:00:00 2001 From: Mingye Wang Date: Sun, 5 Jun 2022 12:28:43 +0800 Subject: [PATCH 4/9] =?UTF-8?q?fixup!=20=E4=B8=BA=E4=BB=80=E4=B9=88?= =?UTF-8?q?=E6=88=91=E8=A6=81=E6=89=93=E5=BC=80=E8=BF=99=E4=B8=AA=E6=82=B2?= =?UTF-8?q?=E4=BC=A4=E7=9A=84=E7=BD=91=E7=AB=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index ce7d507c0c..41989f29cc 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -213,7 +213,7 @@ private static String quoteBatch(String s) { // The argument has not been quoted, add quotes. // See explanation at https://github.com/Artoria2e5/node/blob/fix!/child-process-args/lib/child_process.js // about making the string "inert to CMD", and associated unit tests - return '"' + s.replaceAll("(\\\\*)($|")"", "$1$1$2").replace("\"", "\"\"") + '"'; + return '"' + s.replaceAll("(\\\\*)($|\")\"", "$1$1$2").replace("\"", "\"\"") + '"'; else { return s; } From 0b07cf7d5d53420c7d402cd33be9fefaa9eef031 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 23 Jan 2024 23:01:29 +0800 Subject: [PATCH 5/9] fix build --- .../hmcl/util/platform/CommandBuilder.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index 57a08f00e2..e9ee77c0b4 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -19,7 +19,9 @@ import java.io.BufferedReader; import java.io.InputStreamReader; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import java.util.regex.Matcher; @@ -43,7 +45,7 @@ public CommandBuilder(OperatingSystem os) { this.os = os; } - private String quote(String s) { + private String parse(String s) { if (OperatingSystem.WINDOWS == os) { return toBatchStringLiteral(s); } else { @@ -217,7 +219,7 @@ public boolean noneMatch(Predicate predicate) { @Override public String toString() { - return raw.stream().map(i -> i.quote ? quote(i.arg) : i.arg).collect(Collectors.joining(" ")); + return raw.stream().map(i -> i.parse ? parse(i.arg) : i.arg).collect(Collectors.joining(" ")); } public List asList() { @@ -230,16 +232,16 @@ public List asMutableList() { private static class Item { final String arg; - final boolean quote; + final boolean parse; - Item(String arg, boolean quote) { + Item(String arg, boolean parse) { this.arg = arg; - this.quote = quote; + this.parse = parse; } @Override public String toString() { - return quote ? (OperatingSystem.WINDOWS == OperatingSystem.CURRENT_OS ? toBatchStringLiteral(arg) : toShellStringLiteral(arg)) : arg; + return parse ? (OperatingSystem.WINDOWS == OperatingSystem.CURRENT_OS ? toBatchStringLiteral(arg) : toShellStringLiteral(arg)) : arg; } } @@ -282,6 +284,14 @@ public static boolean setExecutionPolicy() { return true; } + private static boolean containsEscape(String str, String escapeChars) { + for (int i = 0; i < escapeChars.length(); i++) { + if (str.indexOf(escapeChars.charAt(i)) >= 0) + return true; + } + return false; + } + private static String escape(String str, char... escapeChars) { for (char ch : escapeChars) { str = str.replace("" + ch, "\\" + ch); @@ -291,7 +301,7 @@ private static String escape(String str, char... escapeChars) { private static String toBatchStringLiteral(String s) { String escape = " \t\"^&<>|?*"; - if (StringUtils.containsOne(s, escape.toCharArray())) + if (containsEscape(s, escape)) // The argument has not been quoted, add quotes. // See explanation at https://github.com/Artoria2e5/node/blob/fix!/child-process-args/lib/child_process.js // about making the string "inert to CMD", and associated unit tests @@ -301,11 +311,6 @@ private static String toBatchStringLiteral(String s) { } private static String toShellStringLiteral(String s) { - String escape = " \t\"!#$&'()*,;<=>?[\\]^`{|}~"; - if (StringUtils.containsOne(s, escape.toCharArray())) { - return "'" + s.replace("'", "'\''") + "'"; - } else { - return s; - } + return containsEscape(s, " \t\"!#$&'()*,;<=>?[\\]^`{|}~") ? "'" + s.replace("'", "'''") + "'" : s; } } From 87e1f4559845101034563e9890620f5c1a58efb6 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 23 Jan 2024 23:02:08 +0800 Subject: [PATCH 6/9] update --- .../java/org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index e9ee77c0b4..71fb8e1eec 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -54,7 +54,7 @@ private String parse(String s) { } /** - * "Parsing" (quoting) will ignore your manual escaping + * Parsing will ignore your manual escaping * * @param args commands * @return this From 3b9c72211acfd8a2ec58e7a57f60cc3b16349322 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 23 Jan 2024 23:03:37 +0800 Subject: [PATCH 7/9] fix build --- .../java/org/jackhuang/hmcl/launch/DefaultLauncher.java | 8 ++++---- .../org/jackhuang/hmcl/util/platform/CommandBuilder.java | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java index 7bddd2dea5..4ceb822f4d 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java @@ -584,22 +584,22 @@ else if (!isWindows && !scriptExtension.equals("sh")) if (usePowerShell) { if (isWindows) { writer.write("$Env:APPDATA="); - writer.write(CommandBuilder.pwshString(options.getGameDir().getAbsoluteFile().getParent())); + writer.write(CommandBuilder.toPowerShellStringLiteral(options.getGameDir().getAbsoluteFile().getParent())); writer.newLine(); } for (Map.Entry entry : getEnvVars().entrySet()) { writer.write("$Env:" + entry.getKey() + "="); - writer.write(CommandBuilder.pwshString(entry.getValue())); + writer.write(CommandBuilder.toPowerShellStringLiteral(entry.getValue())); writer.newLine(); } writer.write("Set-Location -Path "); - writer.write(CommandBuilder.pwshString(repository.getRunDirectory(version.getId()).getAbsolutePath())); + writer.write(CommandBuilder.toPowerShellStringLiteral(repository.getRunDirectory(version.getId()).getAbsolutePath())); writer.newLine(); writer.write('&'); for (String rawCommand : commandLine.commandLine.asList()) { writer.write(' '); - writer.write(CommandBuilder.pwshString(rawCommand)); + writer.write(CommandBuilder.toPowerShellStringLiteral(rawCommand)); } writer.newLine(); } else { diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index 71fb8e1eec..975e50c82c 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -245,8 +245,7 @@ public String toString() { } } - // Quote for powershell. - public static String toPwshStringLiteral(String str) { + public static String toPowerShellStringLiteral(String str) { return "'" + str.replace("'", "''") + "'"; } @@ -299,7 +298,7 @@ private static String escape(String str, char... escapeChars) { return str; } - private static String toBatchStringLiteral(String s) { + public static String toBatchStringLiteral(String s) { String escape = " \t\"^&<>|?*"; if (containsEscape(s, escape)) // The argument has not been quoted, add quotes. @@ -310,7 +309,7 @@ private static String toBatchStringLiteral(String s) { return s; } - private static String toShellStringLiteral(String s) { + public static String toShellStringLiteral(String s) { return containsEscape(s, " \t\"!#$&'()*,;<=>?[\\]^`{|}~") ? "'" + s.replace("'", "'''") + "'" : s; } } From c1b714d598c6aa74781138552ebe94fa47903494 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 23 Jan 2024 23:05:56 +0800 Subject: [PATCH 8/9] update --- .../java/org/jackhuang/hmcl/launch/DefaultLauncher.java | 8 ++++---- .../org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java index 4ceb822f4d..7bddd2dea5 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java @@ -584,22 +584,22 @@ else if (!isWindows && !scriptExtension.equals("sh")) if (usePowerShell) { if (isWindows) { writer.write("$Env:APPDATA="); - writer.write(CommandBuilder.toPowerShellStringLiteral(options.getGameDir().getAbsoluteFile().getParent())); + writer.write(CommandBuilder.pwshString(options.getGameDir().getAbsoluteFile().getParent())); writer.newLine(); } for (Map.Entry entry : getEnvVars().entrySet()) { writer.write("$Env:" + entry.getKey() + "="); - writer.write(CommandBuilder.toPowerShellStringLiteral(entry.getValue())); + writer.write(CommandBuilder.pwshString(entry.getValue())); writer.newLine(); } writer.write("Set-Location -Path "); - writer.write(CommandBuilder.toPowerShellStringLiteral(repository.getRunDirectory(version.getId()).getAbsolutePath())); + writer.write(CommandBuilder.pwshString(repository.getRunDirectory(version.getId()).getAbsolutePath())); writer.newLine(); writer.write('&'); for (String rawCommand : commandLine.commandLine.asList()) { writer.write(' '); - writer.write(CommandBuilder.toPowerShellStringLiteral(rawCommand)); + writer.write(CommandBuilder.pwshString(rawCommand)); } writer.newLine(); } else { diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index 975e50c82c..c3276ab5a8 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -245,7 +245,7 @@ public String toString() { } } - public static String toPowerShellStringLiteral(String str) { + public static String pwshString(String str) { return "'" + str.replace("'", "''") + "'"; } From b2b1e2fe668da054e72d0489d37c68f226f45071 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 23 Jan 2024 23:27:03 +0800 Subject: [PATCH 9/9] update --- .../java/org/jackhuang/hmcl/util/platform/CommandBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java index c3276ab5a8..6f864d75c4 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java @@ -310,6 +310,6 @@ public static String toBatchStringLiteral(String s) { } public static String toShellStringLiteral(String s) { - return containsEscape(s, " \t\"!#$&'()*,;<=>?[\\]^`{|}~") ? "'" + s.replace("'", "'''") + "'" : s; + return containsEscape(s, " \t\"!#$&'()*,;<=>?[\\]^`{|}~") ? '"' + escape(s, '"', '$', '&', '`') + '"' : s; } }