Merge pull request #13065 from roberth/escapeShellArg

Rename `shellEscape` -> `escapeShellArgAlways`

src/libstore/parsed-derivations.cc

@@ -112,7 +112,7 @@ std::string StructuredAttrs::writeShell(const nlohmann::json & json)
 
     auto handleSimpleType = [](const nlohmann::json & value) -> std::optional<std::string> {
         if (value.is_string())
-            return shellEscape(value.get<std::string_view>());
+            return escapeShellArgAlways(value.get<std::string_view>());
 
         if (value.is_number()) {
             auto f = value.get<float>();
@@ -160,7 +160,7 @@ std::string StructuredAttrs::writeShell(const nlohmann::json & json)
             for (auto & [key2, value2] : value.items()) {
                 auto s3 = handleSimpleType(value2);
                 if (!s3) { good = false; break; }
-                s2 += fmt("[%s]=%s ", shellEscape(key2), *s3);
+                s2 += fmt("[%s]=%s ", escapeShellArgAlways(key2), *s3);
             }
 
             if (good)

src/libutil/include/nix/util/util.hh

@@ -152,8 +152,13 @@ std::string toLower(std::string s);
 
 /**
  * Escape a string as a shell word.
+ *
+ * This always adds single quotes, even if escaping is not strictly necessary.
+ * So both
+ * - `"hello world"` -> `"'hello world'"`, which needs escaping because of the space
+ * - `"echo"` -> `"'echo'"`, which doesn't need escaping
  */
-std::string shellEscape(const std::string_view s);
+std::string escapeShellArgAlways(const std::string_view s);
 
 
 /**

src/libutil/util.cc

@@ -171,7 +171,7 @@ std::string toLower(std::string s)
 }
 
 
-std::string shellEscape(const std::string_view s)
+std::string escapeShellArgAlways(const std::string_view s)
 {
     std::string r;
     r.reserve(s.size() + 2);

src/nix-build/nix-build.cc

@@ -255,16 +255,16 @@ static void main_nix_build(int argc, char * * argv)
 
             std::ostringstream joined;
             for (const auto & i : savedArgs)
-                joined << shellEscape(i) << ' ';
+                joined << escapeShellArgAlways(i) << ' ';
 
             if (std::regex_search(interpreter, std::regex("ruby"))) {
                 // Hack for Ruby. Ruby also examines the shebang. It tries to
                 // read the shebang to understand which packages to read from. Since
                 // this is handled via nix-shell -p, we wrap our ruby script execution
                 // in ruby -e 'load' which ignores the shebangs.
-                envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined));
+                envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, escapeShellArgAlways(script), toView(joined));
             } else {
-                envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined));
+                envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, escapeShellArgAlways(script), toView(joined));
             }
         }
 
@@ -637,12 +637,12 @@ static void main_nix_build(int argc, char * * argv)
                 "unset TZ; %6%"
                 "shopt -s execfail;"
                 "%7%",
-                shellEscape(tmpDir.path().string()),
+                escapeShellArgAlways(tmpDir.path().string()),
                 (pure ? "" : "p=$PATH; "),
                 (pure ? "" : "PATH=$PATH:$p; unset p; "),
-                shellEscape(dirOf(*shell)),
+                escapeShellArgAlways(dirOf(*shell)),
-                shellEscape(*shell),
+                escapeShellArgAlways(*shell),
-                (getenv("TZ") ? (std::string("export TZ=") + shellEscape(getenv("TZ")) + "; ") : ""),
+                (getenv("TZ") ? (std::string("export TZ=") + escapeShellArgAlways(getenv("TZ")) + "; ") : ""),
                 envCommand);
         vomit("Sourcing nix-shell with file %s and contents:\n%s", rcfile, rc);
         writeFile(rcfile, rc);

src/nix-store/nix-store.cc

@@ -497,7 +497,7 @@ static void opPrintEnv(Strings opFlags, Strings opArgs)
     /* Print each environment variable in the derivation in a format
      * that can be sourced by the shell. */
     for (auto & i : drv.env)
-        logger->cout("export %1%; %1%=%2%\n", i.first, shellEscape(i.second));
+        logger->cout("export %1%; %1%=%2%\n", i.first, escapeShellArgAlways(i.second));
 
     /* Also output the arguments.  This doesn't preserve whitespace in
        arguments. */
@@ -506,7 +506,7 @@ static void opPrintEnv(Strings opFlags, Strings opArgs)
     for (auto & i : drv.args) {
         if (!first) cout << ' ';
         first = false;
-        cout << shellEscape(i);
+        cout << escapeShellArgAlways(i);
     }
     cout << "'\n";
 }

src/nix/develop.cc

@@ -156,20 +156,20 @@ struct BuildEnvironment
         for (auto & [name, value] : vars) {
             if (!ignoreVars.count(name)) {
                 if (auto str = std::get_if<String>(&value)) {
-                    out << fmt("%s=%s\n", name, shellEscape(str->value));
+                    out << fmt("%s=%s\n", name, escapeShellArgAlways(str->value));
                     if (str->exported)
                         out << fmt("export %s\n", name);
                 }
                 else if (auto arr = std::get_if<Array>(&value)) {
                     out << "declare -a " << name << "=(";
                     for (auto & s : *arr)
-                        out << shellEscape(s) << " ";
+                        out << escapeShellArgAlways(s) << " ";
                     out << ")\n";
                 }
                 else if (auto arr = std::get_if<Associative>(&value)) {
                     out << "declare -A " << name << "=(";
                     for (auto & [n, v] : *arr)
-                        out << "[" << shellEscape(n) << "]=" << shellEscape(v) << " ";
+                        out << "[" << escapeShellArgAlways(n) << "]=" << escapeShellArgAlways(v) << " ";
                     out << ")\n";
                 }
             }
@@ -614,7 +614,7 @@ struct CmdDevelop : Common, MixEnvironment
             std::vector<std::string> args;
             args.reserve(command.size());
             for (const auto & s : command)
-                args.push_back(shellEscape(s));
+                args.push_back(escapeShellArgAlways(s));
             script += fmt("exec %s\n", concatStringsSep(" ", args));
         }
 
@@ -622,13 +622,13 @@ struct CmdDevelop : Common, MixEnvironment
             script = "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;\nshopt -u expand_aliases\n" + script + "\nshopt -s expand_aliases\n";
             if (developSettings.bashPrompt != "")
                 script += fmt("[ -n \"$PS1\" ] && PS1=%s;\n",
-                    shellEscape(developSettings.bashPrompt.get()));
+                    escapeShellArgAlways(developSettings.bashPrompt.get()));
             if (developSettings.bashPromptPrefix != "")
                 script += fmt("[ -n \"$PS1\" ] && PS1=%s\"$PS1\";\n",
-                    shellEscape(developSettings.bashPromptPrefix.get()));
+                    escapeShellArgAlways(developSettings.bashPromptPrefix.get()));
             if (developSettings.bashPromptSuffix != "")
                 script += fmt("[ -n \"$PS1\" ] && PS1+=%s;\n",
-                    shellEscape(developSettings.bashPromptSuffix.get()));
+                    escapeShellArgAlways(developSettings.bashPromptSuffix.get()));
         }
 
         writeFull(rcFileFd.get(), script);