prefs: Fix serialisation format

We use maybe variants as prefs, so the prefs bridge needs to be able to
send and receive them.  Change from "av" to "ay" format containing the
raw serialisation data of the variants.

Fixes: https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/7935

6 files changed, 110 insertions, 73 deletions

diff --git a/src/org.gnome.Terminal.SettingsBridge.xml b/src/org.gnome.Terminal.SettingsBridge.xml
index 4dac6d78..247ebbe4 100644
--- a/src/org.gnome.Terminal.SettingsBridge.xml
+++ b/src/org.gnome.Terminal.SettingsBridge.xml
@@ -35,13 +35,17 @@
       <arg type="s" name="key" direction="in" />
       <arg type="s" name="type" direction="in" />
       <arg type="b" name="default" direction="in" />
-      <arg type="av" name="value" direction="out" />
+      <arg type="ay" name="value" direction="out">
+        <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true" />
+      </arg>
     </method>
 
     <method name="read_user_value">
       <arg type="s" name="key" direction="in" />
       <arg type="s" name="type" direction="in" />
-      <arg type="av" name="value" direction="out" />
+      <arg type="ay" name="value" direction="out">
+        <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true" />
+      </arg>
     </method>
 
     <method name="reset">
@@ -61,13 +65,15 @@
 
     <method name="write">
       <arg type="s" name="key" direction="in" />
-      <arg type="av" name="value" direction="in" />
+      <arg type="ay" name="value" direction="in">
+        <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true" />
+      </arg>
       <arg type="b" name="success" direction="out" />
     </method>
 
     <method name="write_tree">
       <arg type="s" name="path_prefix" direction="in" />
-      <arg type="a(sav)" name="tree" direction="in">
+      <arg type="ay" name="tree" direction="in">
         <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true" />
       </arg>
       <arg type="b" name="success" direction="out" />
diff --git a/src/terminal-libgsystem.hh b/src/terminal-libgsystem.hh
index 86e3c1fd..370cb286 100644
--- a/src/terminal-libgsystem.hh
+++ b/src/terminal-libgsystem.hh
@@ -67,6 +67,7 @@ GS_DEFINE_CLEANUP_FUNCTION0(GSettingsSchemaKey*, gs_local_settings_schema_key_un
 GS_DEFINE_CLEANUP_FUNCTION0(GVariant*, gs_local_variant_unref, g_variant_unref)
 GS_DEFINE_CLEANUP_FUNCTION0(GVariantBuilder*, gs_local_variant_builder_unref, g_variant_builder_unref)
 GS_DEFINE_CLEANUP_FUNCTION0(GVariantIter*, gs_local_variant_iter_free, g_variant_iter_free)
+GS_DEFINE_CLEANUP_FUNCTION0(GVariantType*, gs_local_variant_type_free, g_variant_type_free)
 
 GS_DEFINE_CLEANUP_FUNCTION(char**, gs_local_strfreev, g_strfreev)
 GS_DEFINE_CLEANUP_FUNCTION(void*, gs_local_free, g_free)
@@ -121,6 +122,14 @@ static inline void gs_local_gstring_free (void *v) \
 #define gs_unref_variant_builder __attribute__ ((cleanup(gs_local_variant_builder_unref)))
 
 /**
+ * gs_free_variant_type:
+ *
+ * Call g_variant_type_free() on a variable location when it goes out of
+ * scope.
+ */
+#define gs_free_variant_type __attribute__ ((cleanup(gs_local_variant_type_free)))
+
+/**
  * gs_unref_array:
  *
  * Call g_array_unref() on a variable location when it goes out of
diff --git a/src/terminal-settings-bridge-backend.cc b/src/terminal-settings-bridge-backend.cc
index 2a6f2a13..4e950de0 100644
--- a/src/terminal-settings-bridge-backend.cc
+++ b/src/terminal-settings-bridge-backend.cc
@@ -23,6 +23,7 @@
 #include "terminal-debug.hh"
 #include "terminal-libgsystem.hh"
 #include "terminal-settings-bridge-backend.hh"
+#include "terminal-settings-utils.hh"
 #include "terminal-settings-bridge-generated.h"
 
 #include <gio/gio.h>
@@ -175,25 +176,6 @@ cache_remove_writable(TerminalSettingsBridgeBackend* impl,
   ce->writable_set = false;
 }
 
-static auto
-wrap(GVariant* value) noexcept
-{
-  auto builder = GVariantBuilder{};
-  g_variant_builder_init(&builder, G_VARIANT_TYPE("av"));
-  if (value)
-    g_variant_builder_add(&builder, "v", value);
-  return g_variant_builder_end(&builder);
-}
-
-static auto
-unwrap(GVariant* value) noexcept
-{
-  auto iter = GVariantIter{};
-  g_variant_iter_init(&iter, value);
-  gs_unref_variant auto cv = g_variant_iter_next_value(&iter);
-  return cv ? g_variant_get_variant(cv) : nullptr;
-}
-
 /* GSettingsBackend class implementation */
 
 static GPermission*
@@ -260,7 +242,7 @@ terminal_settings_bridge_backend_read(GSettingsBackend* backend,
                                             impl->cancellable,
                                             nullptr);
 
-  auto const value = r ? unwrap(rv) : nullptr;
+  auto const value = r ? terminal_g_variant_unwrap(rv) : nullptr;
 
   if (r && value && !g_variant_is_of_type(value, type)) {
     _terminal_debug_print(TERMINAL_DEBUG_BRIDGE,
@@ -288,7 +270,7 @@ terminal_settings_bridge_backend_read(GSettingsBackend* backend,
 static GVariant*
 terminal_settings_bridge_backend_read_user_value(GSettingsBackend* backend,
                                                  char const* key,
-                                                 const GVariantType* type) noexcept
+                                                 GVariantType const* type) noexcept
 {
   auto const impl = IMPL(backend);
 
@@ -305,7 +287,7 @@ terminal_settings_bridge_backend_read_user_value(GSettingsBackend* backend,
                                                        impl->cancellable,
                                                        nullptr);
 
-  auto const value = r ? unwrap(rv) : nullptr;
+  auto const value = r ? terminal_g_variant_unwrap(rv) : nullptr;
 
   if (r && value && !g_variant_is_of_type(value, type)) {
     _terminal_debug_print(TERMINAL_DEBUG_BRIDGE,
@@ -405,7 +387,7 @@ terminal_settings_bridge_backend_write(GSettingsBackend* backend,
   auto const r =
     terminal_settings_bridge_call_write_sync(impl->bridge,
                                              key,
-                                             wrap(value),
+                                             terminal_g_variant_wrap(value),
                                              &success,
                                              impl->cancellable,
                                              nullptr);
@@ -437,14 +419,14 @@ terminal_settings_bridge_backend_write_tree(GSettingsBackend* backend,
                                   &values);
 
   auto builder = GVariantBuilder{};
-  g_variant_builder_init(&builder, G_VARIANT_TYPE("a(sav)"));
+  g_variant_builder_init(&builder, G_VARIANT_TYPE("a(smv)"));
   for (auto i = 0; keys[i]; ++i) {
     gs_unref_variant auto value = values[i] ? g_variant_ref_sink(values[i]) : nullptr;
 
     g_variant_builder_add(&builder,
-                          "(s@av)",
+                          "(smv)",
                           keys[i],
-                          wrap(value));
+                          value ? g_variant_new_variant(value) : nullptr);
 
     gs_free auto wkey = g_strconcat(path_prefix, keys[i], nullptr);
     // Directory reset?
@@ -456,11 +438,13 @@ terminal_settings_bridge_backend_write_tree(GSettingsBackend* backend,
     }
   }
 
+  auto const tree_value = terminal_g_variant_wrap(g_variant_builder_end(&builder));
+
   auto success = gboolean{false};
   auto const r =
     terminal_settings_bridge_call_write_tree_sync(impl->bridge,
                                                   path_prefix,
-                                                  g_variant_builder_end(&builder),
+                                                  tree_value, // consumed
                                                   &success,
                                                   impl->cancellable,
                                                   nullptr);
diff --git a/src/terminal-settings-bridge-impl.cc b/src/terminal-settings-bridge-impl.cc
index ec387fd1..da26e381 100644
--- a/src/terminal-settings-bridge-impl.cc
+++ b/src/terminal-settings-bridge-impl.cc
@@ -46,10 +46,6 @@ struct _TerminalSettingsBridgeImplClass {
   TerminalSettingsBridgeSkeletonClass parent_class;
 };
 
-// Note that since D-Bus doesn't support maybe values, we use
-// arrays with either zero or one item to send/receive a maybe.
-// If we get more than one item, just use the first one.
-
 /* helper functions */
 
 template<typename T>
@@ -76,21 +72,6 @@ type_from_string(GDBusMethodInvocation* invocation,
 }
 
 static auto
-unwrap(GVariantIter* iter) noexcept
-{
-  gs_unref_variant auto iv = g_variant_iter_next_value(iter);
-  return iv ? g_variant_get_variant(iv) : nullptr;
-}
-
-static auto
-unwrap(GVariant* value) noexcept
-{
-  auto iter = GVariantIter{};
-  g_variant_iter_init(&iter, value);
-  return unwrap(&iter);
-}
-
-static auto
 value(GDBusMethodInvocation* invocation,
       char const* format,
       ...) noexcept
@@ -104,18 +85,6 @@ value(GDBusMethodInvocation* invocation,
 }
 
 static auto
-wrap(GDBusMethodInvocation* invocation,
-     GVariant* variant) noexcept
-{
-  auto builder = GVariantBuilder{};
-  g_variant_builder_init(&builder, G_VARIANT_TYPE("av"));
-  if (variant)
-    g_variant_builder_add(&builder, "v", variant);
-
-  return value(invocation, "(av)", &builder);
-}
-
-static auto
 nothing(GDBusMethodInvocation* invocation) noexcept
 {
   return value(invocation, "()");
@@ -177,7 +146,7 @@ terminal_settings_bridge_impl_read(TerminalSettingsBridge* object,
                         "Bridge impl ::read key %s type %s default %d\n",
                         key, type, default_value);
 
-  auto const vtype = type_from_string(invocation, type);
+  gs_free_variant_type auto vtype = type_from_string(invocation, type);
   if (!vtype)
     return true;
 
@@ -186,8 +155,7 @@ terminal_settings_bridge_impl_read(TerminalSettingsBridge* object,
                                                              key,
                                                              vtype,
                                                              default_value);
-  g_variant_type_free(vtype);
-  return wrap(invocation, v);
+  return value(invocation, "(@ay)", terminal_g_variant_wrap(v));
 }
 
 static gboolean
@@ -200,7 +168,7 @@ terminal_settings_bridge_impl_read_user_value(TerminalSettingsBridge* object,
                         "Bridge impl ::read_user_value key %s type %s\n",
                         key, type);
 
-  auto const vtype = type_from_string(invocation, type);
+  gs_free_variant_type auto vtype = type_from_string(invocation, type);
   if (!vtype)
     return true;
 
@@ -208,8 +176,7 @@ terminal_settings_bridge_impl_read_user_value(TerminalSettingsBridge* object,
   gs_unref_variant auto v = terminal_g_settings_backend_read_user_value(impl->backend,
                                                                         key,
                                                                         vtype);
-  g_variant_type_free(vtype);
-  return wrap(invocation, v);
+  return value(invocation, "(@ay)", terminal_g_variant_wrap(v));
 }
 
 static gboolean
@@ -273,7 +240,7 @@ terminal_settings_bridge_impl_write(TerminalSettingsBridge* object,
                                     GVariant* value) noexcept
 {
   auto const impl = IMPL(object);
-  gs_unref_variant auto v = unwrap(value);
+  gs_unref_variant auto v = terminal_g_variant_unwrap(value);
 
   _terminal_debug_print(TERMINAL_DEBUG_BRIDGE,
                         "Bridge impl ::write key %s value %s\n",
@@ -290,23 +257,39 @@ static gboolean
 terminal_settings_bridge_impl_write_tree(TerminalSettingsBridge* object,
                                          GDBusMethodInvocation* invocation,
                                          char const* path_prefix,
-                                         GVariant* tree_value) noexcept
+                                         GVariant* variant) noexcept
 {
   _terminal_debug_print(TERMINAL_DEBUG_BRIDGE,
                         "Bridge impl ::write_tree path-prefix %s\n",
                         path_prefix);
 
+  gs_unref_variant auto tree_value = terminal_g_variant_unwrap(variant);
+  if (!tree_value ||
+      !g_variant_is_of_type(tree_value, G_VARIANT_TYPE("a(smv)"))) {
+    _terminal_debug_print(TERMINAL_DEBUG_BRIDGE,
+                          "Bridge impl ::write_tree got type %s expected type a(smv)\n",
+                          tree_value ? g_variant_get_type_string(tree_value) : "(null)");
+
+    g_dbus_method_invocation_return_error
+      (invocation,
+       G_DBUS_ERROR,
+       G_DBUS_ERROR_INVALID_ARGS,
+       "Invalid type: got type \"%s\" expected type \"a(smv)\"",
+       tree_value ? g_variant_get_type_string(tree_value) : "(null)");
+    return true;
+  }
+
   auto const tree = terminal_g_settings_backend_create_tree();
 
   auto iter = GVariantIter{};
   g_variant_iter_init(&iter, tree_value);
 
   char const* key = nullptr;
-  GVariantIter* viter = nullptr;
-  while (g_variant_iter_loop(&iter, "(&sav)", &key, &viter)) {
+  GVariant* value = nullptr;
+  while (g_variant_iter_loop(&iter, "(&smv)", &key, &value)) {
     g_tree_insert(tree,
                   g_strconcat(path_prefix, key, nullptr), // adopts
-                  unwrap(viter)); // adopts
+                  value ? g_variant_get_variant(value) : nullptr);
   }
 
   auto const impl = IMPL(object);
diff --git a/src/terminal-settings-utils.cc b/src/terminal-settings-utils.cc
index 494a257e..0dd4ff67 100644
--- a/src/terminal-settings-utils.cc
+++ b/src/terminal-settings-utils.cc
@@ -938,4 +938,55 @@ terminal_g_settings_backend_sync(GSettingsBackend* backend)
 
 // END copied from glib
 
+// Note: Since D-Bus/GDBus does not support GVariant maybe types (not even
+// on private peer-to-peer connections), we need to wrap the variants
+// for transport over the bus.  The format is a "mv" variant whose inner
+// value is the variant to transport, or Nothing for a nullptr GVariant.
+// We then transport that variant in serialised form as a byte array over
+// the bus.
+
+/*
+ * terminal_g_variant_wrap:
+ * @variant: (nullable): a #GVariant, or %NULL
+ *
+ * Wraps @variant for transport over D-Bus.
+ * if @variant is floating, it is consumed.
+ *
+ * Returns: (transfer full): a floating variant wrapping @variant
+ */
+GVariant*
+terminal_g_variant_wrap(GVariant* variant)
+{
+  auto const value = variant ? g_variant_new_variant(variant) : nullptr;
+  auto const maybe = g_variant_new_maybe(G_VARIANT_TYPE_VARIANT, value);
+
+  return g_variant_new_from_data(G_VARIANT_TYPE("ay"),
+                                 g_variant_get_data(maybe),
+                                 g_variant_get_size(maybe),
+                                 false, // trusted
+                                 GDestroyNotify(g_variant_unref),
+                                 g_variant_ref_sink(maybe)); // adopts
+}
+
+/*
+ * terminal_g_variant_unwrap:
+ * @variant: a "ay" #GVariant
+ *
+ * Unwraps a variant transported over D-Bus.
+ * If @variant is floating, it is NOT consumed.
+ *
+ * Returns: (transfer full): a non-floating variant unwrapping @variant
+ */
+GVariant*
+terminal_g_variant_unwrap(GVariant* variant)
+{
+  g_return_val_if_fail(g_variant_is_of_type(variant, G_VARIANT_TYPE("ay")), nullptr);
+
+  gs_unref_bytes auto bytes = g_variant_get_data_as_bytes(variant);
+  gs_unref_variant auto maybe = g_variant_new_from_bytes(G_VARIANT_TYPE("mv"), bytes, false);
+  gs_unref_variant auto value = g_variant_get_maybe(maybe);
+  return value ? g_variant_get_variant(value) : nullptr;
+}
+
+
 #endif /* TERMINAL_SERVER || TERMINAL_PREFERENCES */
diff --git a/src/terminal-settings-utils.hh b/src/terminal-settings-utils.hh
index 8a200424..b34afb33 100644
--- a/src/terminal-settings-utils.hh
+++ b/src/terminal-settings-utils.hh
@@ -108,3 +108,7 @@ gboolean terminal_g_settings_backend_write_tree(GSettingsBackend* backend,
                                                 void* origin_tag);
 
 // END copied from glib
+
+GVariant* terminal_g_variant_wrap(GVariant* variant);
+
+GVariant* terminal_g_variant_unwrap(GVariant* variant);