From 75c122699aa9b111b20124af29873762479c6cff Mon Sep 17 00:00:00 2001
From: Gabriella Gonzalez <GenuineGabriella@gmail.com>
Date: Fri, 16 Aug 2024 16:26:08 +0200
Subject: [PATCH] lib.cli.escapeShellArg{,s}: Only escape when necessary
 (#333744)

These utilities will now leave the string undisturbed if it doesn't need to be quoted (because it doesn't have any special characters). This can help generate nicer-looking command lines.

This also transitively improves the output of `lib.toGNUCommandLine` which uses `escapeShellArg` internally
---
 lib/strings.nix    | 16 ++++++++++++----
 lib/tests/misc.nix | 28 ++++++++++++++++++++++++----
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/lib/strings.nix b/lib/strings.nix
index aafbdffaa7bc..54a5b9c67434 100644
--- a/lib/strings.nix
+++ b/lib/strings.nix
@@ -1026,7 +1026,8 @@ rec {
     replaceStrings (builtins.attrNames toEscape) (lib.mapAttrsToList (_: c: "%${fixedWidthString 2 "0" (lib.toHexString c)}") toEscape);
 
   /**
-    Quote `string` to be used safely within the Bourne shell.
+    Quote `string` to be used safely within the Bourne shell if it has any
+    special characters.
 
 
     # Inputs
@@ -1051,10 +1052,17 @@ rec {
 
     :::
   */
-  escapeShellArg = arg: "'${replaceStrings ["'"] ["'\\''"] (toString arg)}'";
+  escapeShellArg = arg:
+    let
+      string = toString arg;
+    in
+      if match "[[:alnum:],._+:@%/-]+" string == null
+      then "'${replaceStrings ["'"] ["'\\''"] string}'"
+      else string;
 
   /**
-    Quote all arguments to be safely passed to the Bourne shell.
+    Quote all arguments that have special characters to be safely passed to the
+    Bourne shell.
 
     # Inputs
 
@@ -1073,7 +1081,7 @@ rec {
 
     ```nix
     escapeShellArgs ["one" "two three" "four'five"]
-    => "'one' 'two three' 'four'\\''five'"
+    => "one 'two three' 'four'\\''five'"
     ```
 
     :::
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index d59f5586b82d..7445c634558c 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -470,6 +470,26 @@ runTests {
     expected = [ "A" "B" ];
   };
 
+  testEscapeShellArg = {
+    expr = strings.escapeShellArg "esc'ape\nme";
+    expected = "'esc'\\''ape\nme'";
+  };
+
+  testEscapeShellArgEmpty = {
+    expr = strings.escapeShellArg "";
+    expected = "''";
+  };
+
+  testEscapeShellArgs = {
+    expr = strings.escapeShellArgs ["one" "two three" "four'five"];
+    expected = "one 'two three' 'four'\\''five'";
+  };
+
+  testEscapeShellArgsUnicode = {
+    expr = strings.escapeShellArg "á";
+    expected = "'á'";
+  };
+
   testSplitStringsDerivation = {
     expr = take 3  (strings.splitString "/" (derivation {
       name = "name";
@@ -569,12 +589,12 @@ runTests {
     '';
     expected = ''
       STRing01='just a '\'''string'\''''
-      declare -a _array_=('with' 'more strings')
+      declare -a _array_=(with 'more strings')
       declare -A assoc=(['with some']='strings
       possibly newlines
       ')
-      drv='/drv'
-      path='/path'
+      drv=/drv
+      path=/path
       stringable='hello toString'
     '';
   };
@@ -1754,7 +1774,7 @@ runTests {
       verbose = true;
     };
 
-    expected = "'-X' 'PUT' '--data' '{\"id\":0}' '--retry' '3' '--url' 'https://example.com/foo' '--url' 'https://example.com/bar' '--verbose'";
+    expected = "-X PUT --data '{\"id\":0}' --retry 3 --url https://example.com/foo --url https://example.com/bar --verbose";
   };
 
   testSanitizeDerivationNameLeadingDots = testSanitizeDerivationName {