Escape tilde more gracefully

comius · copybara-github · commit 30f6c8238f39 · 2022-10-27T05:24:55.000-07:00
Tilde needs to be escaped only when it's the first character after the white space. Otherwise, we can keep the string unescaped. This supports bzlmod better, because tilde is used in the directory names. Users often already escape location function, for example `SJ="$(location @bazel_tools//tools/jdk:singlejar)"; $SJ`. Without the change this becomes `'external/repo~name/singlejar'` and the script fails (no file found). Location function shouldn't be escaped. But without this change we risk a lot of users will need to fix their scripts. Closes bazelbuild#16560. PiperOrigin-RevId: 484226256 Change-Id: I6b71f89a649f8494b76a4446b8f6384421eb89d1
diff --git a/src/main/java/com/google/devtools/build/lib/util/ShellEscaper.java b/src/main/java/com/google/devtools/build/lib/util/ShellEscaper.java
@@ -63,6 +63,8 @@ public final class ShellEscaper extends Escaper {
           .or(CharMatcher.inRange('a', 'z')) // that would also accept non-ASCII digits and
           .or(CharMatcher.inRange('A', 'Z')) // letters.
           .precomputed();
+  private static final CharMatcher SAFECHAR_MATCHER_WITH_TILDE =
+      SAFECHAR_MATCHER.or(CharMatcher.is('~')).precomputed();
 
   /**
    * Escapes a string by adding strong (single) quotes around it if necessary.
@@ -98,9 +100,13 @@ public String escape(String unescaped) {
       // gets treated as a separate argument.
       return "''";
     } else {
-      return SAFECHAR_MATCHER.matchesAllOf(s)
-          ? s
-          : "'" + STRONGQUOTE_ESCAPER.escape(s) + "'";
+      if (SAFECHAR_MATCHER.matchesAllOf(s)) {
+        return s;
+      }
+      if (SAFECHAR_MATCHER_WITH_TILDE.matchesAllOf(s) && s.charAt(0) != '~') {
+        return s;
+      }
+      return "'" + STRONGQUOTE_ESCAPER.escape(s) + "'";
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/util/ShellEscaperTest.java b/src/test/java/com/google/devtools/build/lib/util/ShellEscaperTest.java
@@ -41,6 +41,11 @@ public void shellEscape() throws Exception {
     assertThat(escapeString("\\'foo\\'")).isEqualTo("'\\'\\''foo\\'\\'''");
     assertThat(escapeString("${filename%.c}.o")).isEqualTo("'${filename%.c}.o'");
     assertThat(escapeString("<html!>")).isEqualTo("'<html!>'");
+    assertThat(escapeString("~not_home")).isEqualTo("'~not_home'");
+    assertThat(escapeString("external/protobuf~3.19.6/src/google"))
+        .isEqualTo("external/protobuf~3.19.6/src/google");
+    assertThat(escapeString("external/~install_dev_dependencies~foo/pkg"))
+        .isEqualTo("external/~install_dev_dependencies~foo/pkg");
   }
 
   @Test
