Libguestfs - Jul 2022 - [PATCH] Add simple parsing logic for hostname file.

There are some linux systems in the wild where the /etc/hostname file
does not contain the configured hostname as sole line in that file. This
file actually allows for comments [1] starting with "#" and the
original
logic would extract the comment line instead of the hostname on such
systems. This patch fixes this by filtering out the comment lines and
any empty lines.

[1] https://www.freedesktop.org/software/systemd/man/hostname.html
---
Please keep in mind that this is my first attempt at OCaml after taking
a quick crash-course to get familiar with the language to produce this
patch. Therefore, the patch may be very suboptimal and break guidelines,
so please be forgiving :-)

 daemon/inspect_fs_unix.ml | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
index 63cb279d0..533ef71b2 100644
--- a/daemon/inspect_fs_unix.ml
+++ b/daemon/inspect_fs_unix.ml
@@ -556,11 +556,23 @@ and check_hostname_from_file filename    let chroot     
let name = sprintf "check_hostname_from_file: %s" filename in
     Chroot.create ~name () in
-
-  let hostname = Chroot.f chroot read_small_file filename in
-  match hostname with
-  | None | Some [] | Some [""] -> None
-  | Some (hostname :: _) -> Some hostname
+  try
+    let lines = Chroot.f chroot read_small_file filename in
+    let lines = match lines with
+    | None -> raise Not_found
+    | Some lines -> List.map String.trim lines in
+    let rec loop = function
+      | [] -> raise Not_found
+      | line :: _ when String.length line > 0 &&
+                not (String.is_prefix line "#") ->
+         line
+      | _ :: lines ->
+         loop lines
+    in
+    let hostname = loop lines in
+    Some hostname
+  with
+    Not_found -> None
 
 (* Parse the hostname from /etc/sysconfig/network.  This must be
  * called from the 'with_augeas' wrapper.  Note that F18+ and
-- 
2.36.1

How about this instead?

It actually changes the behaviour slightly.  Currently a /etc/hostname
which contains the single character '\n' would return
inspect_get_hostname = "" (which seems wrong).  This is the default
/etc/hostname for virt-builder images.

I think this is because in the current code, read_small_file actually
returns ?Some [""; ""]?, so it takes the branch where it
matches
?| Some (hostname :: _)? which returns empty string.

Anyway, after this patch it returns "unknown" which is documented as
"the hostname could not be determined", which I think is more correct
behaviour.  I tested it also on files with blank lines and comments
and so on, and it seems to do what is necessary, although it could do
with a proper regression test.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
-------------- next part --------------
>From 3b6897c8ba9c8d438e8599af93188e65cced6ec8 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Wed, 20 Jul 2022 12:33:09 +0100
Subject: [PATCH] daemon: Parse /etc/hostname files containing comments

Thanks: Dawid Zamirski
Link: https://www.freedesktop.org/software/systemd/man/hostname.html
---
 daemon/inspect_fs_unix.ml | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
index 63cb279d08..8045ef0d43 100644
--- a/daemon/inspect_fs_unix.ml
+++ b/daemon/inspect_fs_unix.ml
@@ -551,15 +551,28 @@ and check_hostname_linux ()       else
        None
 
-(* Parse the hostname where it is stored directly in a file. *)
+(* Parse the hostname where it is stored directly in a file.
+ *
+ * For /etc/hostname:
+ * "The file should contain a single newline-terminated hostname
+ * string. Comments (lines starting with a "#") are ignored."
+ * [https://www.freedesktop.org/software/systemd/man/hostname.html]
+ *
+ * For other hostname files the exact format is not clear, but
+ * hostnames cannot begin with "#" and cannot be empty, so ignoring
+ * those lines seems safe.
+ *)
 and check_hostname_from_file filename    let chroot      let name = sprintf
"check_hostname_from_file: %s" filename in
     Chroot.create ~name () in
 
   let hostname = Chroot.f chroot read_small_file filename in
-  match hostname with
-  | None | Some [] | Some [""] -> None
+
+  let keep_line line = line <> "" && not
(String.is_prefix line "#") in
+  let lines = Option.map (List.filter keep_line) hostname in
+  match lines with
+  | None | Some [] -> None
   | Some (hostname :: _) -> Some hostname
 
 (* Parse the hostname from /etc/sysconfig/network.  This must be
-- 
2.37.0.rc2