Merge pull request #109 from ascopes/bugfix/GH-79

GH-79: Handle special characters in shell scripts better
ascopes · Mar 7, 2024 · 3b8a61b · 3b8a61b
2 parents 313d79c + ca2edf7
commit 3b8a61b
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 9 deletions.
diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
@@ -17,6 +17,12 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
+      - name: Run ShellCheck
+        uses: ludeeus/action-shellcheck@master
+        with:
+          check_together: 'yes'
+          scandir: './scripts'
+
       - name: Validate codecov.yml
         shell: bash
         run: curl -vvv --fail --data-binary @- https://codecov.io/validate < codecov.yml
diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/platform/Shlex.java b/src/main/java/io/github/ascopes/protobufmavenplugin/platform/Shlex.java
@@ -22,6 +22,10 @@
  *
  * <p>Losely based on Python's {@code shlex} module.
  *
+ * <p>This is far from perfect but should work in the majority of use cases
+ * to ensure scripts do not interpret special characters in paths in strange
+ * and unexpected ways.
+ *
  * @author Ashley Scopes
  */
 public final class Shlex {
@@ -42,6 +46,7 @@ private static String quote(Iterable<String> args, BiConsumer<StringBuilder, Str
     var iter = args.iterator();
 
     if (!iter.hasNext()) {
+      // Probably won't ever happen.
       return "";
     }
 
@@ -65,10 +70,22 @@ private static void quoteShellArg(StringBuilder sb, String arg) {
     sb.append('\'');
     for (var i = 0; i < arg.length(); ++i) {
       var c = arg.charAt(i);
-      if (c == '\'') {
-        sb.append("'\"'\"'");
-      } else {
-        sb.append(c);
+      switch (c) {
+        case '\'':
+          sb.append("'\"'\"'");
+          break;
+        case '\n':
+          sb.append("'$'\\n''");
+          break;
+        case '\r':
+          sb.append("'$'\\r''");
+          break;
+        case '\t':
+          sb.append("'$'\\t''");
+          break;
+        default:
+          sb.append(c);
+          break;
       }
     }
     sb.append('\'');
@@ -83,6 +100,9 @@ private static void quoteBatchArg(StringBuilder sb, String arg) {
     for (var i = 0; i < arg.length(); ++i) {
       var c = arg.charAt(i);
       switch (c) {
+        case '%':
+          sb.append("%%");
+          break;
         case '\\':
         case '"':
         case '\'':
@@ -94,10 +114,12 @@ private static void quoteBatchArg(StringBuilder sb, String arg) {
         case '<':
         case '>':
         case '|':
-          sb.append('^');
+          sb.append('^').append(c);
+          break;
+        default:
+          sb.append(c);
+          break;
       }
-
-      sb.append(c);
     }
   }
 

diff --git a/src/test/java/io/github/ascopes/protobufmavenplugin/platform/ShlexTest.java b/src/test/java/io/github/ascopes/protobufmavenplugin/platform/ShlexTest.java
@@ -81,7 +81,10 @@ static Stream<Arguments> quoteShellArgsTestCases() {
         arguments(list("\\"), "'\\'"),
         arguments(list("\""), "'\"'"),
         arguments(list("po'tato"), "'po'\"'\"'tato'"),
-        arguments(list("'potato'"), "''\"'\"'potato'\"'\"''")
+        arguments(list("'potato'"), "''\"'\"'potato'\"'\"''"),
+        arguments(list("foo\nbar", "baz"), "'foo'$'\\n''bar' baz"),
+        arguments(list("foo\rbar", "baz"), "'foo'$'\\r''bar' baz"),
+        arguments(list("foo\tbar", "baz"), "'foo'$'\\t''bar' baz")
     );
   }
 
@@ -110,7 +113,8 @@ static Stream<Arguments> quoteBatchArgsTestCases() {
         arguments(list("&"), "^&"),
         arguments(list("<"), "^<"),
         arguments(list(">"), "^>"),
-        arguments(list("|"), "^|")
+        arguments(list("|"), "^|"),
+        arguments(list("100% complete", "0% incomplete"), "100%%^ complete 0%%^ incomplete")
     );
   }