Correctly serialize exports in service rumors

components/butterfly/src/rumor/service.rs

@@ -87,7 +87,7 @@ impl Service {
         package: &T,
         service_group: ServiceGroup,
         sys: SysInfo,
-        cfg: Option<&toml::value::Table>,
+        cfg: Option<toml::value::Table>,
     ) -> Self
     where
         T: Identifiable,
@@ -110,10 +110,14 @@ impl Service {
             initialized: false,
             pkg: package.to_string(),
             sys: sys,
-            // TODO FN: Can we really expect this all the time, should we return a `Result<Self>`
-            // in this constructor?
             cfg: cfg
-                .map(|v| toml::ser::to_vec(v).expect("Struct should serialize to bytes"))
+                .map(|v| {
+                    // Directly serializing a toml::value::Table can lead to an error
+                    // Wrapping it in a toml::value::Value makes this operation safe
+                    // See https://github.com/alexcrichton/toml-rs/issues/142
+                    toml::ser::to_vec(&toml::value::Value::Table(v))
+                        .expect("Struct should serialize to bytes")
+                })
                 .unwrap_or_default(),
         }
     }
@@ -352,4 +356,21 @@ mod tests {
             None,
         );
     }
+
+    #[test]
+    fn service_cfg_serialization() {
+        let package: PackageIdent = "core/foo/1.0.0/20180701125610".parse().unwrap();
+        let sg = ServiceGroup::new(None, "foo", "default", None).unwrap();
+
+        // This map contains a scalar value and a table such that the serialization order
+        // would trigger a ValueAfterTable error. This test ensures we avoid it.
+        // See https://github.com/habitat-sh/habitat/issues/5854
+        // See https://github.com/alexcrichton/toml-rs/issues/142
+        let mut map = toml::value::Table::default();
+        let sub_map = toml::value::Table::default();
+        map.insert("foo".into(), 5.into());
+        map.insert("a".into(), sub_map.into());
+
+        Service::new("member_id_val", &package, sg, SysInfo::default(), Some(map));
+    }
 }

components/sup/src/manager/service/config.rs

@@ -24,7 +24,6 @@ use std::result;
 use crate::fs;
 use crate::hcore::fs::USER_CONFIG_FILE;
 use crate::hcore::{self, crypto};
-use serde::ser::SerializeMap;
 use serde::{Serialize, Serializer};
 use serde_json;
 use serde_transcode;
@@ -226,20 +225,19 @@ impl Cfg {
         }
     }
 
-    /// Returns a subset of the overall configuration which intersects with the given package's exports.
+    /// Returns a subset of the overall configuration which intersects with the given package exports.
     pub fn to_exported(&self, pkg: &Pkg) -> Result<toml::value::Table> {
         let mut map = toml::value::Table::default();
-        let cfg = toml::Value::try_from(&self).unwrap();
+        let cfg = toml::Value::try_from(&self).expect("Cfg -> TOML conversion");;
         for (key, path) in pkg.exports.iter() {
-            let fields: Vec<&str> = path.split('.').collect();
             let mut curr = &cfg;
             let mut found = false;
 
             // JW TODO: the TOML library only provides us with a
             // function to retrieve a value with a path which returns a
             // reference. We actually want the value for ourselves.
             // Let's improve this later to avoid allocating twice.
-            for field in fields {
+            for field in path.split('.') {
                 match curr.get(field) {
                     Some(val) => {
                         curr = val;
@@ -435,28 +433,7 @@ impl Serialize for Cfg {
             }
         }
 
-        // Be sure to visit non-tables first (and also non
-        // array-of-tables) as all keys must be emitted first.
-        let mut map = serializer.serialize_map(Some(table.len()))?;
-        for (k, v) in &table {
-            if !v.is_array() && !v.is_table() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
-        }
-        for (k, v) in &table {
-            if v.is_array() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
-        }
-        for (k, v) in &table {
-            if v.is_table() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
-        }
-        map.end()
+        toml::ser::tables_last(&table, serializer)
     }
 }
 
@@ -1075,5 +1052,4 @@ mod test {
             }
         }
     }
-
 }

components/sup/src/manager/service/mod.rs

@@ -681,7 +681,7 @@ impl Service {
             &self.pkg.ident,
             self.service_group.clone(),
             self.sys.as_sys_info().clone(),
-            exported.as_ref(),
+            exported,
         );
         rumor.incarnation = incarnation;
         rumor