🔧 Support <includedir>

A little less efficient than @elmarco 's implementation over in dbus2#23

src/config.rs

@@ -93,7 +93,12 @@ impl TryFrom<Document> for Config {
                     bc.auth.insert(s);
                 }
                 Element::Fork => bc.fork = true,
-                Element::Include(_) => { /* no-op */ }
+                Element::Include(_) => {
+                    // NO-OP: removed during `Document::resolve_includes`
+                }
+                Element::Includedir(_) => {
+                    // NO-OP: removed during `Document::resolve_includedirs`
+                }
                 Element::KeepUmask => bc.keep_umask = true,
                 Element::Limit => {
                     warn!("busd does not implement `<limit>`");

src/config/xml.rs

@@ -1,6 +1,7 @@
 use std::{
     env::current_dir,
-    fs::read_to_string,
+    ffi::OsString,
+    fs::{read_dir, read_to_string},
     path::{Path, PathBuf},
     str::FromStr,
 };
@@ -36,7 +37,64 @@ impl Document {
 
         let mut doc = Document::from_str(&text)?;
         doc.file_path = Some(file_path.as_ref().to_path_buf());
-        doc.resolve_includes()
+        doc.resolve_includedirs()?.resolve_includes()
+    }
+
+    fn resolve_includedirs(self) -> Result<Document> {
+        let base_path = self.base_path()?;
+        let Document {
+            busconfig,
+            file_path,
+        } = self;
+
+        let mut doc = Document {
+            busconfig: vec![],
+            file_path: None,
+        };
+
+        for el in busconfig {
+            match el {
+                Element::Includedir(dir_path) => {
+                    let dir_path = resolve_include_path(&base_path, &dir_path);
+                    let dir_path = match dir_path.canonicalize() {
+                        Ok(ok) => ok,
+                        // we treat `<includedir>` as though it has `ignore_missing="yes"`
+                        Err(err) => {
+                            warn!(
+                                "should canonicalize directory path '{}': {:?}",
+                                &dir_path.display(),
+                                err
+                            );
+                            continue;
+                        }
+                    };
+                    match read_dir(&dir_path) {
+                        Ok(ok) => {
+                            for entry in ok {
+                                let path = entry?.path();
+                                if path.extension() == Some(&OsString::from("conf"))
+                                    && path.is_file()
+                                {
+                                    doc.busconfig.push(Element::Include(IncludeElement {
+                                        file_path: path,
+                                        ..Default::default()
+                                    }));
+                                }
+                            }
+                        }
+                        // we treat `<includedir>` as though it has `ignore_missing="yes"`
+                        Err(err) => {
+                            warn!("should read directory '{}': {:?}", &dir_path.display(), err);
+                            continue;
+                        }
+                    }
+                }
+                _ => doc.busconfig.push(el),
+            }
+        }
+
+        doc.file_path = file_path;
+        Ok(doc)
     }
 
     fn resolve_includes(self) -> Result<Document> {
@@ -56,12 +114,14 @@ impl Document {
             match el {
                 Element::Include(include) => {
                     let ignore_missing = include.ignore_missing == IncludeOption::Yes;
-                    let file_path = match resolve_include_path(&base_path, &include.file_path) {
+                    let file_path = resolve_include_path(&base_path, &include.file_path);
+                    let file_path = match file_path.canonicalize().map_err(Error::msg) {
                         Ok(ok) => ok,
                         Err(err) => {
                             let msg = format!(
-                                "'{}' should be a valid file path",
-                                include.file_path.display()
+                                "should canonicalize file path '{}': {:?}",
+                                &file_path.display(),
+                                err
                             );
                             if ignore_missing {
                                 warn!(msg);
@@ -117,7 +177,7 @@ pub enum Element {
     Auth(String),
     Fork,
     Include(IncludeElement),
-    // TODO: support `<includedir>`
+    Includedir(PathBuf),
     KeepUmask,
     Listen(String),
     Limit,
@@ -255,13 +315,10 @@ pub struct TypeElement {
     pub r#type: BusType,
 }
 
-fn resolve_include_path(
-    base_path: impl AsRef<Path>,
-    include_path: impl AsRef<Path>,
-) -> Result<PathBuf> {
+fn resolve_include_path(base_path: impl AsRef<Path>, include_path: impl AsRef<Path>) -> PathBuf {
     let p = include_path.as_ref();
     if p.is_absolute() {
-        return p.canonicalize().map_err(Error::msg);
+        return p.to_path_buf();
     }
 
     error!(
@@ -270,9 +327,5 @@ fn resolve_include_path(
         &include_path.as_ref().display()
     );
 
-    base_path
-        .as_ref()
-        .join(p)
-        .canonicalize()
-        .map_err(Error::msg)
+    base_path.as_ref().join(p)
 }

tests/config.rs

@@ -12,8 +12,11 @@ fn config_read_file_with_includes_ok() {
         Config {
             auth: HashSet::from_iter(vec![String::from("ANONYMOUS"), String::from("EXTERNAL"),]),
             listen: HashSet::from_iter(vec![
+                String::from("unix:path=/tmp/a"), // via <includedir>
+                String::from("unix:path=/tmp/b"), // via <includedir>
+                // should be no "unix:path=/tmp/not_included" as that file ends in .xml
                 String::from("unix:path=/tmp/foo"),
-                String::from("tcp:host=localhost,port=1234"),
+                String::from("tcp:host=localhost,port=1234"), // via <include>
             ]),
             policies: vec![
                 Policy::DefaultContext(vec![

tests/fixtures/includedir/a.conf

@@ -0,0 +1,5 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
+    "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+    <listen>unix:path=/tmp/a</listen>
+</busconfig>

tests/fixtures/includedir/b.conf

@@ -0,0 +1,5 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
+    "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+    <listen>unix:path=/tmp/b</listen>
+</busconfig>

tests/fixtures/includedir/not_included.xml

@@ -0,0 +1,5 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
+    "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+    <listen>unix:path=/tmp/not_included</listen>
+</busconfig>

tests/fixtures/valid.conf

@@ -9,4 +9,5 @@
     </policy>
     <include>./valid_included.conf</include>
     <include ignore_missing="yes">./valid_missing.conf</include>
+    <includedir>./includedir</includedir>
 </busconfig>