From 19246ac48d0555ed942f73628d61755bd03d0c67 Mon Sep 17 00:00:00 2001 From: Kate Date: Mon, 30 Sep 2024 20:14:00 +0100 Subject: [PATCH] OpamStd.Sys.{get_terminal_columns,uname,getconf,guess_shell_compat}: Harden the process calls to account for failure with_process_in was fairly brittle and would display stderr by default and ignore errors. The new version of that function (renamed process_in) now uses Unix.open_process_full and returns None if anything went wrong, fixing the issue on OpenBSD where `getconf LONG_BIT` would fail. --- master_changes.md | 2 ++ src/core/opamStd.ml | 63 ++++++++++++++++++++++++--------------------- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/master_changes.md b/master_changes.md index a0daabfcf50..69634b12e2f 100644 --- a/master_changes.md +++ b/master_changes.md @@ -126,3 +126,5 @@ users) ## opam-format ## opam-core + * `OpamStd.Sys.{get_terminal_columns,uname,getconf,guess_shell_compat}`: Harden the process calls to account for failures [#6230 @kit-ty-kate - fix #6215] + * `OpamStd.Sys.{uname,getconf}`: now accepts only one argument as parameter, as per their documentation [#6230 @kit-ty-kate] diff --git a/src/core/opamStd.ml b/src/core/opamStd.ml index dcc75638677..0452c306f62 100644 --- a/src/core/opamStd.ml +++ b/src/core/opamStd.ml @@ -903,19 +903,23 @@ module OpamSys = struct let split = if clean then OpamString.split else OpamString.split_delim in split path path_sep - let with_process_in cmd args f = + let process_in cmd args = if Sys.win32 then assert false; - let path = split_path_variable (Env.get "PATH") in - let cmd = - List.find Sys.file_exists (List.map (fun d -> Filename.concat d cmd) path) - in - let ic = Unix.open_process_in (cmd^" "^args) in + let env = Env.raw_env () in try - let r = f ic in - ignore (Unix.close_process_in ic) ; r - with exn -> - ignore (Unix.close_process_in ic) ; raise exn + let path = split_path_variable (Env.get "PATH") in + let cmd = + List.find OpamStubs.is_executable + (List.map (fun d -> Filename.concat d cmd) path) + in + let args = Array.of_list (cmd :: args) in + let (ic, _, _) as p = Unix.open_process_args_full cmd args env in + let r = input_line ic in + match Unix.close_process_full p with + | Unix.WEXITED 0 -> Some r + | WEXITED _ | WSIGNALED _ | WSTOPPED _ -> None + with Unix.Unix_error _ | Sys_error _ | End_of_file | Not_found -> None let tty_out = Unix.isatty Unix.stdout @@ -934,20 +938,21 @@ module OpamSys = struct let get_terminal_columns () = let fallback = 80 in let cols = - try (* terminfo *) - with_process_in "tput" "cols" - (fun ic -> int_of_string (input_line ic)) + match (* terminfo *) + Option.replace int_of_string_opt (process_in "tput" ["cols"]) with - | Unix.Unix_error _ | Sys_error _ | Failure _ | End_of_file | Not_found -> + | Some x -> x + | None -> try (* GNU stty *) - with_process_in "stty" "size" - (fun ic -> - match OpamString.split (input_line ic) ' ' with - | [_ ; v] -> int_of_string v - | _ -> failwith "stty") + begin match + Option.map (fun x -> OpamString.split x ' ') + (process_in "stty" ["size"]) + with + | Some [_ ; v] -> int_of_string v + | _ -> failwith "stty" + end with - | Unix.Unix_error _ | Sys_error _ | Failure _ - | End_of_file | Not_found -> fallback + | Failure _ -> fallback in if cols > 0 then cols else fallback @@ -1000,10 +1005,9 @@ module OpamSys = struct fun cmd arg -> try Hashtbl.find memo (cmd, arg) with Not_found -> let r = - try - with_process_in cmd arg - (fun ic -> Some (OpamString.strip (input_line ic))) - with Unix.Unix_error _ | Sys_error _ | Not_found -> None + match process_in cmd [arg] with + | None -> None + | Some x -> Some (OpamString.strip x) in Hashtbl.add memo (cmd, arg) r; r @@ -1119,12 +1123,11 @@ module OpamSys = struct Some (Unix.readlink (Filename.concat dir "exe")) with e -> fatal e; - try - with_process_in "ps" - (Printf.sprintf "-p %d -o comm= 2>/dev/null" ppid) - (fun ic -> Some (input_line ic)) + match + process_in "ps" ["-p"; string_of_int ppid; "-o"; "comm="] with - | Unix.Unix_error _ | Sys_error _ | Failure _ | End_of_file | Not_found -> + | Some _ as x -> x + | None -> try let c = open_in_bin ("/proc/" ^ string_of_int ppid ^ "/cmdline") in begin try