BUGFIX: add the interface type if given when creating the proxy

[drivera73]

The interface type value "_type" is already null if not given as a parameter. But if given, it's being ignored while constructing the RemoteObject reference that will eventually be proxied. This leads to method invocations failing with "strange" errors (inapplicable instances of "access denied", for instance).

This correction caused the methods to at least be invoked correctly.

[hypfvieh]

Thanks for the PR. It would be great if you could provide a unit test case for this issue. I see the bug, but a test case will ensure that this bug will not be re-introduced in the future by any change.

[drivera73]

I'll try to do that today so we can get this merged. I also found an issue where FDs are meant to be supported, but aren't actually (the filedescriptors list is always null when invoking MessageFactory.createMessage():

// from InputStreamMessage.java, line 163
final Message m = MessageFactory.createMessage(type, buf, header, body, null);

But I suppose this underlies a larger issue about how to properly handle those ... I'm currently looking into how python does it. But this is a separate issue :)

[drivera73]

As promised, the FD issue is described here, but is now moot :)

[drivera73]

In other news, I've written this test class:

/**
  * The package must be "both" org.freedesktop.dbus.connections.impl and org.freedesktop.dbus in order
  * for the test to be able to access the requisite methods and fields... obviously this is an issue...
  */
package org.freedesktop.dbus.test;

import java.lang.reflect.Proxy;

import org.freedesktop.dbus.RemoteInvocationHandler;
import org.freedesktop.dbus.RemoteObject;
import org.freedesktop.dbus.bin.EmbeddedDBusDaemon;
import org.freedesktop.dbus.connections.BusAddress;
import org.freedesktop.dbus.connections.impl.DBusConnection;
import org.freedesktop.dbus.connections.impl.DBusConnectionBuilder;
import org.freedesktop.dbus.connections.transports.TransportBuilder;
import org.freedesktop.dbus.interfaces.DBusInterface;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class InterfaceCreationTest extends AbstractBaseTest {

    public static interface TestInterfaceType extends DBusInterface {

    }

    @Test
    public void testCorrectInterfaceCreation() throws Exception {
        String protocolType = TransportBuilder.getRegisteredBusTypes().get(0);
        BusAddress busAddress = TransportBuilder.createWithDynamicSession(protocolType).configure().build()
            .getBusAddress();

        BusAddress listenBusAddress = BusAddress.of(busAddress).getListenerAddress();

        try (EmbeddedDBusDaemon daemon = new EmbeddedDBusDaemon(listenBusAddress)) {
            daemon.startInBackground();
            this.logger.debug("Started embedded bus on address {}", listenBusAddress);

            waitForDaemon(daemon);

            // connect to started daemon process
            this.logger.info("Connecting to embedded DBus {}", busAddress);

            final String source = "org.freedesktop";
            final String path = "/org/freedesktop";

            try (DBusConnection connection = DBusConnectionBuilder.forAddress(busAddress).build()) {
                DBusInterface dbi = null;
                RemoteInvocationHandler rih = null;
                RemoteObject ro = null;
                Class<? extends DBusInterface> type = TestInterfaceType.class;

                // DBusConnection.dynamicProxy is protected scope, but the class is final ...
                dbi = connection.dynamicProxy(source, path, null);
                Assertions.assertNotNull(dbi);

                rih = RemoteInvocationHandler.class.cast(Proxy.getInvocationHandler(dbi));
                Assertions.assertNotNull(rih);

                // RemoteInvocationHandler.remote is package scope
                ro = rih.remote;

                Assertions.assertNull(ro.getInterface());

                // DBusConnection.dynamicProxy is protected scope, but the class is final ...
                dbi = connection.dynamicProxy(source, path, type);
                Assertions.assertNotNull(dbi);
                Assertions.assertTrue(TestInterfaceType.class.isInstance(dbi));

                rih = RemoteInvocationHandler.class.cast(Proxy.getInvocationHandler(dbi));
                Assertions.assertNotNull(rih);

                // RemoteInvocationHandler.remote is package scope
                ro = rih.remote;

                Assertions.assertNotNull(ro.getInterface());
                Assertions.assertSame(type, ro.getInterface());
            }
        }
    }

}

The leading comment describes the issue here. It's impossible to implement this test without code/scope modifications to the methods/fields listed - at least one of them must be made public, which I don't think is appropriate.

The problem children are:

• field: org.freedesktop.dbus.RemoteInvocationHandler.remote (is package-scoped)
• method: org.freedesktop.dbus.connections.impl.DBusConnection.dynamicProxy(String, String, Class<DBusInterface>) (is protected-scoped, and the declaring class is final)

Thoughts/ideas?

[drivera73]

Closing this one in favor of one that includes a working test.