Libguestfs - Sep 2023 - [PATCH libguestfs] daemon: Omit 'file -S' option on older distros that lack support

OpenSUSE LEAP 15 lacks support for this option, so test for it before
using it.

See-also:
https://listman.redhat.com/archives/libguestfs/2023-September/032613.html
Report-by: Olaf Hering
Fixes: commit 23986d3c4f4d1f9cbac44cc743d3e6af721e4237
---
 daemon/Makefile.am     |  2 ++
 daemon/file.ml         | 10 ++++++++--
 daemon/file_helper.ml  | 29 +++++++++++++++++++++++++++++
 daemon/file_helper.mli | 19 +++++++++++++++++++
 daemon/filearch.ml     |  5 ++++-
 5 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index bb2e58d014..01c0f6416c 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -280,6 +280,7 @@ SOURCES_MLI = \
 	devsparts.mli \
 	file.mli \
 	filearch.mli \
+	file_helper.mli \
 	findfs.mli \
 	inspect.mli \
 	inspect_fs.mli \
@@ -321,6 +322,7 @@ SOURCES_ML = \
 	btrfs.ml \
 	cryptsetup.ml \
 	devsparts.ml \
+	file_helper.ml \
 	file.ml \
 	filearch.ml \
 	isoinfo.ml \
diff --git a/daemon/file.ml b/daemon/file.ml
index 1f87b190c1..f0ef181938 100644
--- a/daemon/file.ml
+++ b/daemon/file.ml
@@ -43,7 +43,10 @@ let file path      | S_SOCK -> "socket"
     | S_REG ->
        (* Regular file, so now run [file] on it. *)
-       let out = command "file" ["-zSb";
Sysroot.sysroot_path path] in
+       let file_options +         sprintf "-z%sb"
+           (if File_helper.file_has_S_option () then "S" else
"") in
+       let out = command "file" [file_options; Sysroot.sysroot_path
path] in
 
        (*  We need to remove the trailing \n from output of file(1).
         *
@@ -54,6 +57,9 @@ let file path         String.trimr out
   )
   else (* it's a device *) (
-    let out = command "file" ["-zSbsL"; path] in
+    let file_options +      sprintf "-z%sbsL"
+        (if File_helper.file_has_S_option () then "S" else
"") in
+    let out = command "file" [file_options; path] in
     String.trimr out
   )
diff --git a/daemon/file_helper.ml b/daemon/file_helper.ml
new file mode 100644
index 0000000000..f8c4bcbe56
--- /dev/null
+++ b/daemon/file_helper.ml
@@ -0,0 +1,29 @@
+(* guestfs-inspection
+ * Copyright (C) 2009-2023 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Std_utils
+
+(* Does [file] support the [-S] / [--no-sandbox] option
+ * (not on OpenSUSE LEAP 15).
+ *)
+let file_has_S_option = lazy (
+  let out = Utils.command "file" ["file";
"--help"] in
+  String.find out "--no-sandbox" >= 0
+
+)
+let file_has_S_option () = Lazy.force file_has_S_option
diff --git a/daemon/file_helper.mli b/daemon/file_helper.mli
new file mode 100644
index 0000000000..a644cf6de2
--- /dev/null
+++ b/daemon/file_helper.mli
@@ -0,0 +1,19 @@
+(* guestfs-inspection
+ * Copyright (C) 2009-2023 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+val file_has_S_option : unit -> bool
diff --git a/daemon/filearch.ml b/daemon/filearch.ml
index 7c858129db..cf784f18a2 100644
--- a/daemon/filearch.ml
+++ b/daemon/filearch.ml
@@ -128,7 +128,10 @@ and cpio_arch magic orig_path path          | bin :: bins
->
            let bin_path = tmpdir // bin in
            if is_regular_file bin_path then (
-             let out = command "file" ["-zSb"; bin_path] in
+             let file_options +               sprintf "-z%sb"
+                 (if File_helper.file_has_S_option () then "S" else
"") in
+             let out = command "file" [file_options; bin_path] in
              file_architecture_of_magic out orig_path bin_path
            )
            else
-- 
2.41.0

On 9/21/23 13:42, Richard W.M. Jones wrote:
> OpenSUSE LEAP 15 lacks support for this option, so test for it before
> using it.
> 
> See-also:
https://listman.redhat.com/archives/libguestfs/2023-September/032613.html
> Report-by: Olaf Hering
> Fixes: commit 23986d3c4f4d1f9cbac44cc743d3e6af721e4237
> ---
>  daemon/Makefile.am     |  2 ++
>  daemon/file.ml         | 10 ++++++++--
>  daemon/file_helper.ml  | 29 +++++++++++++++++++++++++++++
>  daemon/file_helper.mli | 19 +++++++++++++++++++
>  daemon/filearch.ml     |  5 ++++-
>  5 files changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
> index bb2e58d014..01c0f6416c 100644
> --- a/daemon/Makefile.am
> +++ b/daemon/Makefile.am
> @@ -280,6 +280,7 @@ SOURCES_MLI = \
>  	devsparts.mli \
>  	file.mli \
>  	filearch.mli \
> +	file_helper.mli \
>  	findfs.mli \
>  	inspect.mli \
>  	inspect_fs.mli \
> @@ -321,6 +322,7 @@ SOURCES_ML = \
>  	btrfs.ml \
>  	cryptsetup.ml \
>  	devsparts.ml \
> +	file_helper.ml \
>  	file.ml \
>  	filearch.ml \
>  	isoinfo.ml \
> diff --git a/daemon/file.ml b/daemon/file.ml
> index 1f87b190c1..f0ef181938 100644
> --- a/daemon/file.ml
> +++ b/daemon/file.ml
> @@ -43,7 +43,10 @@ let file path >      | S_SOCK ->
"socket"
>      | S_REG ->
>         (* Regular file, so now run [file] on it. *)
> -       let out = command "file" ["-zSb";
Sysroot.sysroot_path path] in
> +       let file_options > +         sprintf "-z%sb"
> +           (if File_helper.file_has_S_option () then "S" else
"") in
> +       let out = command "file" [file_options;
Sysroot.sysroot_path path] in
>  
>         (*  We need to remove the trailing \n from output of file(1).
>          *
This still open-codes "-z" and "-b" in multiple places, plus
the
file_has_S_option() call; I thought of a somewhat thicker wrapper
(hiding all the details / options behind the "file" and
"file-architecture" APIs). But, this works as well of course.
> @@ -54,6 +57,9 @@ let file path >         String.trimr out
>    )
>    else (* it's a device *) (
> -    let out = command "file" ["-zSbsL"; path] in
> +    let file_options > +      sprintf "-z%sbsL"
> +        (if File_helper.file_has_S_option () then "S" else
"") in
> +    let out = command "file" [file_options; path] in
>      String.trimr out
>    )
> diff --git a/daemon/file_helper.ml b/daemon/file_helper.ml
> new file mode 100644
> index 0000000000..f8c4bcbe56
> --- /dev/null
> +++ b/daemon/file_helper.ml
> @@ -0,0 +1,29 @@
> +(* guestfs-inspection
"guestfs-inspection" -- strange, but I see it in a bunch of other
places.
> + * Copyright (C) 2009-2023 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *)
> +
> +open Std_utils
> +
> +(* Does [file] support the [-S] / [--no-sandbox] option
> + * (not on OpenSUSE LEAP 15).
> + *)
> +let file_has_S_option = lazy (
> +  let out = Utils.command "file" ["file";
"--help"] in
I think this executes "file file --help", which happens to work, but
the
intent is probably just "file --help". (IOW, IMO this should be

Utils.command "file" ["--help"]

.)
> +  String.find out "--no-sandbox" >= 0
> +
> +)
> +let file_has_S_option () = Lazy.force file_has_S_option
> diff --git a/daemon/file_helper.mli b/daemon/file_helper.mli
> new file mode 100644
> index 0000000000..a644cf6de2
> --- /dev/null
> +++ b/daemon/file_helper.mli
> @@ -0,0 +1,19 @@
> +(* guestfs-inspection
> + * Copyright (C) 2009-2023 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *)
> +
> +val file_has_S_option : unit -> bool
> diff --git a/daemon/filearch.ml b/daemon/filearch.ml
> index 7c858129db..cf784f18a2 100644
> --- a/daemon/filearch.ml
> +++ b/daemon/filearch.ml
> @@ -128,7 +128,10 @@ and cpio_arch magic orig_path path >          | bin
:: bins ->
>             let bin_path = tmpdir // bin in
>             if is_regular_file bin_path then (
> -             let out = command "file" ["-zSb";
bin_path] in
> +             let file_options > +               sprintf
"-z%sb"
> +                 (if File_helper.file_has_S_option () then "S"
else "") in
> +             let out = command "file" [file_options; bin_path]
in
>               file_architecture_of_magic out orig_path bin_path
>             )
>             else
Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks!
Laszlo