mvidner / ruby-dbus

A Ruby binding for DBus
GNU Lesser General Public License v2.1
170 stars 51 forks source link

Wrong type of structs on PropertiesChanged #115

Closed imobachgs closed 2 years ago

imobachgs commented 2 years ago

In D-Installer we have an object with the following properties:

‣ Type=method_return  Endian=l  Flags=1  Version=1 Cookie=191  ReplyCookie=207  Timestamp="Tue 2022-06-14 12:14:39.530914 UTC"
  Sender=:1.1175  Destination=:1.1171
  UniqueName=:1.1175
  MESSAGE "a{sv}" {
          ARRAY "{sv}" {
                  DICT_ENTRY "sv" {
                          STRING "RootPasswordSet";
                          VARIANT "b" {
                                  BOOLEAN false;
                          };
                  };
                  DICT_ENTRY "sv" {
                          STRING "RootSSHKey";
                          VARIANT "s" {
                                  STRING "";
                          };
                  };
                  DICT_ENTRY "sv" {
                          STRING "FirstUser";
                          VARIANT "(ssba{sv})" {
                                  STRUCT "ssba{sv}" {
                                          STRING "";
                                          STRING "user1";
                                          BOOLEAN false;
                                          ARRAY "{sv}" {
                                          };
                                  };
                          };
                  };
          };
  };

However, when we launch a PropertiesChanged signal to change the FirstUser property, the list of changes looks like this:

‣ Type=signal  Endian=l  Flags=0  Version=1 Cookie=215  Timestamp="Tue 2022-06-14 12:14:44.260642 UTC"
  Sender=:1.1175  Path=/org/opensuse/DInstaller/Users1  Interface=org.freedesktop.DBus.Properties  Member=PropertiesChanged
  UniqueName=:1.1175
  MESSAGE "sa{sv}as" {
          STRING "org.opensuse.DInstaller.Users1";
          ARRAY "{sv}" {
                  DICT_ENTRY "sv" {
                          STRING "FirstUser";
                          VARIANT "av" {
                                  ARRAY "v" {
                                          VARIANT "s" {
                                                  STRING "";
                                          };
                                          VARIANT "s" {
                                                  STRING "user1";
                                          };
                                          VARIANT "b" {
                                                  BOOLEAN false;
                                          };
                                          VARIANT "a{sv}" {
                                                  ARRAY "{sv}" {
                                                  };
                                          };
                                  };
                          };
                  };
          };
          ARRAY "s" {
          };
  };

We expected a struct, but that's not the case. The problem can be reproduced by using the D-Installer CLI:

cd service
sudo bundle exec bin/dinstaller &
cd ../cli
bundle exec bin/dinstallerctl user set user1

Could you have a look, please? Thanks!

mvidner commented 2 years ago

I have reproduced the problem using d-installer, will make a reproducer/test-case here.

The problem is that all the current calls of PropertiesChanged treat it as just another method: they use a plain Ruby value of the changed property and the formal parameter of the method is of the variant type. So, the marshalling code ends up having to guess the type.

We need to special-case PropertiesChanged to refer to the specific type of the property, similar to what the client code does in #113.

Also we need to adapt the documentation for DBus::Object to mention the correct usage (and add a helper for it?)

mvidner commented 2 years ago

To reproduce here:

$ SVC="org.ruby.service"; dbus-monitor --session "sender='$SVC'" "destination='$SVC'" &
$ bundle exec spec/service_newapi &
$ busctl --user set-property \
    org.ruby.service /org/ruby/MyInstance org.ruby.SampleInterface MyStruct '(sss)' aa bb cc

method call time=1655217312.613765 sender=:1.1473 -> destination=org.ruby.service serial=2 path=/org/ruby/MyInstance; interface=org.freedesktop.DBus.Properties; member=Set
   string "org.ruby.SampleInterface"
   string "MyStruct"
   variant       struct {
         string "aa"
         string "bb"
         string "cc"
      }
signal time=1655217312.616923 sender=:1.1467 -> destination=(null destination) serial=44 path=/org/ruby/MyInstance; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "org.ruby.SampleInterface"
   array [
      dict entry(
         string "MyStruct"
         variant             array [
               variant                   string "aa"
               variant                   string "bb"
               variant                   string "cc"
            ]
      )
   ]
   array [
   ]

The four lines after string "MyStruct" should be the same in the signal and the method call, but the signal got it wrong.

jreidinger commented 2 years ago

This issue basically blocks https://github.com/yast/d-installer/pull/192 as it does not properly redraw storage proposal after change of product.