Review Request 119540: don't construct bogus KAuthAction objects

[Harald Sitter]

> On Juli 30, 2014, 2:29 nachm., Luca Beltrame wrote:
> > My question is, is KAuth being constructed with a default string ("") useful to anyone? IMO the issue, if any, it's there, and this looks like a workaround. (But I claim no expertise on the code, so feel free to flame me :P)

>From what I understand: since kauth supports multiple backends assuming that an empty string necessarily represents an invalid action may well be wrong, the apidox at the very least do not say anything about this. It does however constitute an invalid action for polkit-qt, so there is a problem in polkit's authority class in that it either should reject operations on an empty action id preventing obvious errors or (somewhat more importantly I guess) recover from having used an invalid action id previously. At a polkit-qt level what happens is that the action results in a (IIRC) instance error which will make all further requests be discarded on account of the Authority having the error flag set, from that point on it does not automatically recover from. So to fix this particular polkit-qt specific issue I can actually imagine two ways this should work a) kauth should use clearError() before trying to do anything b) polkit-qt clearing the error when a new action(id) is requested. I totally do not feel comfortable saying which one it should be but from a convenience POV latter certainly seems best, having to clear error flags from outside the library is not a case I feel is often present in Qt-based libraries.

Regardless of all that, Auth(0) with 0 being implicitly cast to QString causing the Auth(QString&) ctor to be used certainly is not the intended initialization in kcmodule. So it happens to be a workaround for the Authority encountering an error and then refusing to do things until someone clears the error, it also happens to be a fix for using a wrong ctor and weirdly implict casting QString though ;)


- Harald


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119540/#review63509
-----------------------------------------------------------


On Juli 30, 2014, 12:32 vorm., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119540/
> -----------------------------------------------------------
> 
> (Updated Juli 30, 2014, 12:32 vorm.)
> 
> 
> Review request for KDE Frameworks, Hrvoje Senjan and Martin Bříza.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> -------
> 
> KAuthAction(0) actually calls the QString constructor which will dispatch
> bogus polkit auth checks, polkitqt and polkitd are not terribly happy about
> those and subsequently don't want to talk to the KCM anymore, even if it
> manages to create a proper Action
> 
> creating a null Action rather than accidentally triggering the QString
> ctor ensures that we only do polkit interaction with reasonable actionids
> making sure that authentication works as expected
> 
> 
> Diffs
> -----
> 
>   src/kcmodule.cpp 92e5427c121491e4ebf289addda040cc117cdd68 
> 
> Diff: https://git.reviewboard.kde.org/r/119540/diff/
> 
> 
> Testing
> -------
> 
> tested with clock kcm, succesfully can talk with the helper app if the bogus actionid "" wasn't used intermediately
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140730/cc357608/attachment.html>