From cd74e2b8ea4f795f7208dd034b9e1449b653d48b Mon Sep 17 00:00:00 2001 From: Kate Date: Tue, 10 Dec 2024 18:29:16 +0000 Subject: [PATCH] Quote command line arguments if necessary when printing them --- src/core/opamProcess.ml | 22 +++++++++++++++++++--- src/core/opamProcess.mli | 2 ++ src/core/opamSystem.ml | 2 +- src/state/opamSysInteract.ml | 4 ++-- tests/reftests/assume-built.test | 2 +- tests/reftests/env.unix.test | 22 +++++++++++----------- tests/reftests/run.ml | 2 +- tests/reftests/swhid.unix.test | 4 ++-- 8 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/core/opamProcess.ml b/src/core/opamProcess.ml index 2d810728ed2..048f6be8b42 100644 --- a/src/core/opamProcess.ml +++ b/src/core/opamProcess.ml @@ -211,7 +211,23 @@ type command = { cmd_metadata: (string * string) list option; } -let string_of_command c = String.concat " " (c.cmd::c.args) +let string_of_cmd cmd args = + let is_not_shell_special_char = function + | '0'..'9' | 'a'..'z' | 'A'..'Z' + | '+' | '.' | '/' | '@' | '_' -> true + | '\\' -> Sys.win32 + | '%' | ',' | '-' | '^' -> not Sys.win32 + | _ -> false + in + let print x = + if String.for_all is_not_shell_special_char x then + x + else + Filename.quote x + in + print cmd ^ List.fold_left (fun acc x -> acc ^ " " ^ print x) "" args + +let string_of_command c = string_of_cmd c.cmd c.args let text_of_command c = c.cmd_text let default_verbose () = OpamCoreConfig.(!r.verbose_level) >= 2 let is_verbose_command c = @@ -225,7 +241,7 @@ let make_command_text ?(color=`green) str ?(args=[]) cmd = not (String.contains s '/') && not (String.contains s '=')) args with - | hd::_ -> String.concat " " [cmd; hd] + | hd::_ -> string_of_cmd cmd [hd] | [] -> cmd in Printf.sprintf "[%s: %s]" (OpamConsole.colorise color str) summary @@ -281,7 +297,7 @@ let make_info ?code ?signal List.iter (fun (k,v) -> print k v) metadata; print "path" cwd; - print "command" (String.concat " " (cmd :: args)); + print "command" (string_of_cmd cmd args); print_opt "exit-code" (OpamStd.Option.map string_of_int code); print_opt "signalled" (OpamStd.Option.map string_of_int signal); print_opt "env-file" env_file; diff --git a/src/core/opamProcess.mli b/src/core/opamProcess.mli index 8455369e534..ec45387e5c6 100644 --- a/src/core/opamProcess.mli +++ b/src/core/opamProcess.mli @@ -11,6 +11,8 @@ (** Process and job handling, with logs, termination status, etc. *) +val string_of_cmd : string -> string list -> string + (** The type of shell commands *) type command = private { cmd: string; diff --git a/src/core/opamSystem.ml b/src/core/opamSystem.ml index 3ec0735a788..c5c09b65169 100644 --- a/src/core/opamSystem.ml +++ b/src/core/opamSystem.ml @@ -570,7 +570,7 @@ let run_process ~env ~name ~verbose ?metadata ?allow_stdin ?stdout full_cmd args) in - let str = String.concat " " (cmd :: args) in + let str = OpamProcess.string_of_cmd cmd args in log ~level:2 "[%a] (in %.3fs) %s" (OpamConsole.slog Filename.basename) name (chrono ()) str; diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index c8896d06494..46283204c79 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -1117,7 +1117,7 @@ let sudo_run_command ?(env=OpamVariable.Map.empty) ?vars cmd args = | `AsAdmin cmd, Some ("linux" | "unix" | "freebsd" | "netbsd" | "dragonfly" | "macos") when not_root -> if OpamSystem.resolve_command "sudo" = None then "su", - ["root"; "-c"; Printf.sprintf "%S" (String.concat " " (cmd::args))] + ["root"; "-c"; Printf.sprintf "%S" (OpamProcess.string_of_cmd cmd args)] else "sudo", cmd::args | (`AsUser cmd | `AsAdmin cmd), _ -> cmd, args @@ -1127,7 +1127,7 @@ let sudo_run_command ?(env=OpamVariable.Map.empty) ?vars cmd args = | code -> Printf.ksprintf failwith "failed with exit code %d at command:\n %s" - code (String.concat " " (cmd::args)) + code (OpamProcess.string_of_cmd cmd args) let install ?env config packages = if OpamSysPkg.Set.is_empty packages then diff --git a/tests/reftests/assume-built.test b/tests/reftests/assume-built.test index 166c6d810b2..541149bb196 100644 --- a/tests/reftests/assume-built.test +++ b/tests/reftests/assume-built.test @@ -34,7 +34,7 @@ Processing 1/3: [ongoing.dev: git] -> retrieved ongoing.dev (no changes) Processing 2/3: [ongoing: test ongoing.txt] + test "-f" "ongoing.txt" (CWD=${BASEDIR}/OPAM/assume-built/.opam-switch/build/ongoing.dev) -Processing 2/3: [ongoing: sh cat ongoing.txt > out] +Processing 2/3: [ongoing: sh 'cat ongoing.txt > out'] + sh "-c" "cat ongoing.txt > out" (CWD=${BASEDIR}/OPAM/assume-built/.opam-switch/build/ongoing.dev) -> compiled ongoing.dev Processing 3/3: [ongoing: test out] diff --git a/tests/reftests/env.unix.test b/tests/reftests/env.unix.test index 4afdf07ee27..8680586fe6d 100644 --- a/tests/reftests/env.unix.test +++ b/tests/reftests/env.unix.test @@ -70,7 +70,7 @@ The following actions will be performed: - install col-target 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [col-target: sh env | grep RCT_ENV | sort] +Processing 2/3: [col-target: sh 'env | grep RCT_ENV | sort'] + sh "-c" "env | grep RCT_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target.1) - RCT_ENVBUILD=/a/given/path - RCT_ENVBUILD_ADD=/a/given/path:a/path/to @@ -137,7 +137,7 @@ The following actions will be performed: - install col-target-quoted 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [col-target-quoted: sh env | grep RCTQ_ENV | sort] +Processing 2/3: [col-target-quoted: sh 'env | grep RCTQ_ENV | sort'] + sh "-c" "env | grep RCTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target-quoted.1) - RCTQ_ENVBUILD=/a/given/path - RCTQ_ENVBUILD_ADD=/a/given/path:a/path/to @@ -204,7 +204,7 @@ The following actions will be performed: - install col-host 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [col-host: sh env | grep RCH_ENV | sort] +Processing 2/3: [col-host: sh 'env | grep RCH_ENV | sort'] + sh "-c" "env | grep RCH_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host.1) - RCH_ENVBUILD=/a/given/path - RCH_ENVBUILD_ADD=/a/given/path:a/path/to @@ -271,7 +271,7 @@ The following actions will be performed: - install col-host-quoted 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [col-host-quoted: sh env | grep RCHQ_ENV | sort] +Processing 2/3: [col-host-quoted: sh 'env | grep RCHQ_ENV | sort'] + sh "-c" "env | grep RCHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host-quoted.1) - RCHQ_ENVBUILD=/a/given/path - RCHQ_ENVBUILD_ADD=/a/given/path:a/path/to @@ -335,7 +335,7 @@ The following actions will be performed: - install semicol-target 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [semicol-target: sh env | grep RST_ENV | sort] +Processing 2/3: [semicol-target: sh 'env | grep RST_ENV | sort'] + sh "-c" "env | grep RST_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target.1) - RST_ENVBUILD=/a/given/path - RST_ENVBUILD_ADD=/a/given/path;a/path/to @@ -402,7 +402,7 @@ The following actions will be performed: - install semicol-target-quoted 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [semicol-target-quoted: sh env | grep RSTQ_ENV | sort] +Processing 2/3: [semicol-target-quoted: sh 'env | grep RSTQ_ENV | sort'] + sh "-c" "env | grep RSTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target-quoted.1) - RSTQ_ENVBUILD=/a/given/path - RSTQ_ENVBUILD_ADD=/a/given/path;a/path/to @@ -469,7 +469,7 @@ The following actions will be performed: - install semicol-host 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [semicol-host: sh env | grep RSH_ENV | sort] +Processing 2/3: [semicol-host: sh 'env | grep RSH_ENV | sort'] + sh "-c" "env | grep RSH_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host.1) - RSH_ENVBUILD=/a/given/path - RSH_ENVBUILD_ADD=/a/given/path;a/path/to @@ -536,7 +536,7 @@ The following actions will be performed: - install semicol-host-quoted 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [semicol-host-quoted: sh env | grep RSHQ_ENV | sort] +Processing 2/3: [semicol-host-quoted: sh 'env | grep RSHQ_ENV | sort'] + sh "-c" "env | grep RSHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host-quoted.1) - RSHQ_ENVBUILD=/a/given/path - RSHQ_ENVBUILD_ADD=/a/given/path;a/path/to @@ -603,7 +603,7 @@ The following actions will be performed: - install false 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [false: sh env | grep RF_ENV | sort] +Processing 2/3: [false: sh 'env | grep RF_ENV | sort'] + sh "-c" "env | grep RF_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/false.1) - RF_ENVBUILD=/a/given/path - RF_ENVBUILD_ADD=/a/given/path:a/path/to @@ -672,7 +672,7 @@ The following actions will be performed: - install rewrite 1 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -Processing 2/3: [rewrite: sh env | grep RO_ENV | sort] +Processing 2/3: [rewrite: sh 'env | grep RO_ENV | sort'] + sh "-c" "env | grep RO_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/rewrite.1) - RO_ENVBUILD=/another/given/path - RO_ENVBUILD_COL=s:mething @@ -745,7 +745,7 @@ The following actions will be performed: - install all-formulae 1 [ERROR] Formula can't be completely resolved : RAF_ENVBUILD_UNRES ":" true | ";" true. Using default ':' 'target'. [ERROR] Formula can't be completely resolved : RAF_ENVBUILD_UNRES ("host" true | "target-quoted" true). Using default ':' 'target'. -Processing 2/3: [all-formulae: sh env | grep RAF_ENV | sort] +Processing 2/3: [all-formulae: sh 'env | grep RAF_ENV | sort'] + sh "-c" "env | grep RAF_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/all-formulae.1) [ERROR] Formula can't be completely resolved : RAF_ENVSET_UNRES ("host" true | "target-quoted" true). Using default ':' 'target'. [ERROR] Formula can't be completely resolved : RAF_ENVSET_UNRES ":" true | ";" true. Using default ':' 'target'. diff --git a/tests/reftests/run.ml b/tests/reftests/run.ml index 9875ac94a43..0aca95df280 100644 --- a/tests/reftests/run.ml +++ b/tests/reftests/run.ml @@ -207,7 +207,7 @@ let command List.iter print_endline out; let out = String.concat "\n" out in if not (List.mem ret allowed_codes) then - raise (Command_failure (ret, String.concat " " (cmd :: args), out)) + raise (Command_failure (ret, OpamProcess.string_of_cmd cmd args, out)) else out diff --git a/tests/reftests/swhid.unix.test b/tests/reftests/swhid.unix.test index f12f5d32bf6..242830be091 100644 --- a/tests/reftests/swhid.unix.test +++ b/tests/reftests/swhid.unix.test @@ -39,7 +39,7 @@ The following actions will be performed: <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> [ERROR] Failed to get sources of snappy-ko.2: curl failed -OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)") +OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out '%{http_code}\\n' --retry 3 --retry-delay 2 --user-agent 'opam/current' -L -o ${OPAMTMP}/url.tar.gz.part -- 'https://fake.exe/url.tar.gz'\" exited with code 6)") <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> @@ -68,7 +68,7 @@ The following actions will be performed: Processing 1/3: [snappy-swhid-dir.2: http] [ERROR] Failed to get sources of snappy-swhid-dir.2: curl failed -OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)") +OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out '%{http_code}\\n' --retry 3 --retry-delay 2 --user-agent 'opam/current' -L -o ${OPAMTMP}/url.tar.gz.part -- 'https://fake.exe/url.tar.gz'\" exited with code 6)") <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>