[sugar] Initial Security Patches

Simon McVittie simon.mcvittie
Tue Jul 31 09:20:27 EDT 2007


On Mon, 30 Jul 2007 at 16:21:36 -0400, Michael Stone wrote:
>      @dbus.service.signal(_DBUS_OWNER_IFACE, signature="s")
>      def CurrentActivityChanged(self, activity_id):
> -        pass
> +        if os.path.exists('/etc/olpc-security'):
> +            self.rainbow.activity_changed(activity_id)

Minor: It's conventional for D-Bus methods to be named in CamelCase.

More major: This looks like an example of the same method/signal
naming inversion we have in Telepathy's streamed media interfaces (which we
consider to be a bug). Methods should be verbs, like ChangeActivity;
signals should be events, like ActivityChanged. Can't Rainbow just
listen for _DBUS_OWNER_IFACE::CurrentActivityChanged? If you're implementing
Rainbow in Python, that's spelt:

    bus = dbus.SessionBus()
    bus.add_signal_receiver(some_callback, 'CurrentActivityChanged',
                            _DBUS_OWNER_IFACE)

If you only want to respond to CurrentActivityChanged from the process
or object that ought to be emitting it, you can also specify a bus name
or object path, or you can use connect_to_signal() on a proxy for that
object.

If you must use a method call for some technical reason, it'd be less
astonishing if it was called something like ChangeCurrentActivity.

> +            system_bus = dbus.SystemBus()
> +            factory = system_bus.get_object(_RAINBOW_SERVICE_NAME,
> +                                     _RAINBOW_ACTIVITY_FACTORY_PATH)

Proxy objects trigger an Introspect() call every time they're created,
so you may want to cache this proxy object for use in future launches.

> +            factory.create_activity(self._service_name,

Again, this would conventionally be called CreateActivity. It's an
appropriate use for a method this time, though.

	Simon



More information about the Sugar-devel mailing list