Review Request: Fix sanitization of dbus path in KMainWindow

[Matthew Woehlke]

> On 2009-08-11 05:23:13, Chani wrote:
> > how will this affect applications that do have a - in their name? and applications that use the dbus interface of those apps?
> 
> thomasl wrote:
>     As a dbus path attempt of "/some-app/MainWindow_1" is catched by an assertion in Qt (for being invalid) there's never been a chance that you used it this way
>     
>     If we'd however catch the appName in KAboutData to align the dbus path with e.g. qApp->applicationName(), this would -likely- affect at least the KConfig rc path and possibly the way it registers with KGlobalAction :-(
>     
>     (if anyone wonders, i had of course a much cooler "." in my failing app ;-)
>     
>     As the limitiation of the appName would be artificial, one could also choose to change the dbus path system of KMainWindow to e.g. "/MainWindows/MainWindow_1", "/MainWindows/MainWindow_2", etc. - but that would break everything here :-(
> 
> Tom Albers wrote:
>     Concrete question: will:
>     
>      QDBusInterface dbus( "org.kde.plasma-desktop", "/App" );
>     
>     change into 
>     
>      QDBusInterface dbus( "org.kde.plasmadesktop", "/App" );
>     
>     ?
> 
> thomasl wrote:
>     No. "plasma-desktop" is a valid /service/ and "/App" is a valid /path/
>     If plasma-desktop had a mainwindow KMainWindow would (as by Matthews patch) "change" it's /path/ to
>     
>     /plasma_desktop/<objectName()>
>     
>     as
>     
>     /plasma-desktop/<objectName()>
>     
>     would be an invalid dbus path (i didn't write the spec, ok ;-P)
>     
>     fact is, we /need/ to fix up KMainWindow (or have developers figure out that they have to change their appname when adding one)
>     
>     As a consequence we'll live (in the plasma-desktop case) with one of the options:
>     - "/app_name/MainWindow_1" != "/"+qApp->applicationName()+"/MainWindow_1"
>     - qApp->applicationName(): plasma-desktop -> plasma_desktop
>     - /app_name/MainWindow_1 -> /MainWindows/MainWindow_1
>     
>     each introduces some kind of break, where the latter one have external impact and the first (aside the devs need to remember) creates an inconsistency in the way to access the KMainWindow dbus object -> :-(

This patch doesn't affect appName, just objectName. We may want to do something about appName also, but for the moment that is still an open question; please see http://permalink.gmane.org/gmane.comp.kde.devel.general/58753 .

Unless I misunderstand the spec, I don't think this can possibly break anything, since all it does is allow paths that previously would have been invalid (and would have thrown an assert leading to your application not working anyway).


- Matthew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1261/#review1969
-----------------------------------------------------------


On 2009-08-10 22:27:09, Matthew Woehlke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1261/
> -----------------------------------------------------------
> 
> (Updated 2009-08-10 22:27:09)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> As pointed out by Thomas Lübking ( http://permalink.gmane.org/gmane.comp.kde.devel.general/58749 ), KMainWindow attempts to sanitize what it will allow in the dbus path. However as written it would allow the illegal characters "." and "-" to be passed through.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/widgets/kmainwindow.cpp 1009802 
> 
> Diff: http://reviewboard.kde.org/r/1261/diff
> 
> 
> Testing
> -------
> 
> built kdelibs and ran a KDE application
> 
> 
> Thanks,
> 
> Matthew
> 
>