hypfvieh
commented
6 years ago Would you please share some parts of your code, so I can try to reproduce this issue myself?
Closed haggish closed 6 years ago
hypfvieh
commented
6 years ago Would you please share some parts of your code, so I can try to reproduce this issue myself?
haggish
commented
6 years ago Please see the commited unit test in my fork, if you debug it and change the offset array input for second extractOne call from [10, 8] to [9, 8], it will serialize the example message body properly.
hypfvieh
commented
6 years ago I also want to try it against connman. All my tries right now resulted in a different error than yours, so please show me your implementation of ServicesChangeHandler.
haggish
commented
6 years ago When I am monitoring Connman DBus traffic with gdbus monitor --system --dest net.connman:
/: net.connman.Manager.ServicesChanged (@a(oa{sv}) [], [objectpath '/net/connman/service/wifi_80d21d5f4569_52494f_managed_psk', '/net/connman/service/wifi_80d21d5f4569_4755455354_managed_none'])(seems legit?)
At approximately same time I hit a breakpoint that deserializes the parameters to
result = {Object[2]@5358}
0 = {Vector@5359} size = 0
1 = {ObjectPath@5360} "9 /net/connman/service/wifi_80d21d5f4569_52494f_managed_psk > /net/connman/service/wifi_80d21d5f4569_4755455354_managed_none "The handler (inner class):
private class ServicesChangeHandler implements DBusSigHandler<IDBusConnmanManager.ServicesChanged> {
@Override
public void handle(IDBusConnmanManager.ServicesChanged s) {
// ...
}
}Signal (inner class), this is being constructed when getting the CCE:
class ServicesChanged extends DBusSignal {
public final String path;
public final List<PathProperties> changedServices;
public final List<Path> removedServices;
public ServicesChanged(String path, List<PathProperties> changedServices, List<Path> removedServices)
throws DBusException {
super(path, changedServices, removedServices);
this.path = path;
this.changedServices = changedServices;
this.removedServices = removedServices;
}
}This is really a tough one. Changing the extractone method to fix your issue, will then break the usage of structs (TestAll Unit test will fail).
I'll take a deeper look into it. Maybe the whole parsing stuff should be re-designed.
hypfvieh
commented
6 years ago So I think I got the solution.
After digging around in the Message and Marshalling class, I think there is nothing wrong.
The mistake in my view is in your PathProperties class (the part of your code, you did not publish).
PathProperties have to extend Struct and has to use the Position annotation.
The provided unit test fails, because using Message.extract directly behaves different then using the Marshalling.deSerializeParameters.
If you need custom classes like in this case (a holder which extends Struct), Message.extract will not work, as it does not know about your custom class.
You need to use Marshalling.deSerializeParameters and provide the required parameters.
I updated the unit test to do so and also implemented a proper 'Struct', so unit test is now fine. Also my short tests with connman in a virtual machine are working like expected.
For completeness, here is my test code I used to talk to connman:
import java.util.List;
import java.util.Map;
import org.freedesktop.dbus.DBusPath;
import org.freedesktop.dbus.ObjectPath;
import org.freedesktop.dbus.Struct;
import org.freedesktop.dbus.annotations.DBusInterfaceName;
import org.freedesktop.dbus.annotations.Position;
import org.freedesktop.dbus.connections.impl.DBusConnection;
import org.freedesktop.dbus.connections.impl.DBusConnection.DBusBusType;
import org.freedesktop.dbus.exceptions.DBusException;
import org.freedesktop.dbus.handlers.AbstractSignalHandlerBase;
import org.freedesktop.dbus.interfaces.DBusInterface;
import org.freedesktop.dbus.messages.DBusSignal;
import org.freedesktop.dbus.types.Variant;
public class TestConnman {
public static void main(String[] args) throws DBusException, InterruptedException {
DBusConnection connection = DBusConnection.getConnection(DBusBusType.SYSTEM);
connection.addSigHandler(IServicesChanged.ServicesChanged.class, new ServiceChangedHandler() {
@Override
public void handle(IServicesChanged.ServicesChanged _signal) {
System.out.print("Received signal.");
System.out.println("Removed devices: " + _signal.getRemoved());
System.out.println("Changed properties: " + someDataToString(_signal.getChanged()));
}
private String someDataToString(List<SomeData> changed) {
StringBuilder sb = new StringBuilder();
for (SomeData string : changed) {
sb.append("DbusPath: ").append(string.getObjectPath()).append(" - Properties: ")
.append(string.getProperties()).append("\n");
}
return sb.toString();
}
});
Thread.sleep(100000);
}
public abstract static class ServiceChangedHandler
extends AbstractSignalHandlerBase<IServicesChanged.ServicesChanged> {
@Override
public Class<IServicesChanged.ServicesChanged> getImplementationClass() {
return IServicesChanged.ServicesChanged.class;
}
}
@DBusInterfaceName("net.connman.Manager")
interface IServicesChanged extends DBusInterface {
public static class ServicesChanged extends DBusSignal {
public final String objectPath;
public final List<SomeData> changed;
public final List<ObjectPath> removed;
public ServicesChanged(String _objectPath, List<SomeData> _k, List<ObjectPath> _removedItems)
throws DBusException {
super(_objectPath, _k, _removedItems);
objectPath = _objectPath;
changed = _k;
removed = _removedItems;
}
public String getObjectPath() {
return objectPath;
}
public List<SomeData> getChanged() {
return changed;
}
public List<ObjectPath> getRemoved() {
return removed;
}
}
}
public static class SomeData extends Struct {
@Position(0)
public DBusPath objectPath;
@Position(1)
public Map<String, Variant<?>> properties;
public SomeData(DBusPath objectPath, Map<String, Variant<?>> properties) {
this.objectPath = objectPath;
this.properties = properties;
}
DBusPath getObjectPath() {
return objectPath;
}
void setObjectPath(DBusPath _objectPath) {
objectPath = _objectPath;
}
Map<String, Variant<?>> getProperties() {
return properties;
}
void setProperties(Map<String, Variant<?>> _properties) {
properties = _properties;
}
}
}
littlefreaky
commented
6 years ago I had the exact same problem. The issue seems to be that, if the changedServices list is empty, the optimizePrimitives method in the Message class wants to skip the signature of the struct (Message:1054). The length of the struct signature is calculated by using Marshalling.getJavaType, which returns a size that is one off for structs if I read it correctly. See my small fix above.
I am listening to some signals from DBus, one of them is "ServicesChanged" from Connman, which has a signature of
a(oa{sv})ao.It seems that the signals that have the first array as empty, and the other array (of object paths) non-empty, result in a
ClassCastException:What seems to happen is that while getting the signal constructor parameters in
Message.getParameters(), the array of object paths is interpreted as one object path (the one path value is all the object path strings mangled together plus some garbage inbetween). Then when giving the parameters to Reflection, an exception is thrown as the wrongly deserialized constructor param isObjectPathand notList<ObjectPath>.Specifically, after having parsed the empty first array, the second round of
extractOnemethod for the non-empty object path array is given an offset array which seems to have its first element (signature offset) one element too far and this makes the method try to parse an array of object paths as one. Hence the first round ofextractOneof the two seems to update the offset to a value one too many. When I debug and set the signature offset to a value one less forextractOneafter extracting the empty array, the parameters are deserialized correctly.This is happening in (little-endian) signals with this exact signature, in 2.7.* and 3.0 both.