From 30f6c8238f39c4a396b3cb56a98c1a2e79d10bb9 Mon Sep 17 00:00:00 2001
From: Ivo List <ilist@google.com>
Date: Thu, 27 Oct 2022 05:23:27 -0700
Subject: [PATCH] Escape tilde more gracefully

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 #16560.

PiperOrigin-RevId: 484226256
Change-Id: I6b71f89a649f8494b76a4446b8f6384421eb89d1
---
 .../google/devtools/build/lib/util/ShellEscaper.java | 12 +++++++++---
 .../devtools/build/lib/util/ShellEscaperTest.java    |  5 +++++
 2 files changed, 14 insertions(+), 3 deletions(-)

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
index d54bbdaf8e5c0f..121fbb8284006d 100644
--- 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
index cad9ce53f24add..d40473b24dc8eb 100644
--- 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