Re: Review Request 119540: don't construct bogus KAuthAction objects
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119540/#review63630 --- Ship it! Ship It! - Lukáš Tinkl On Čec. 30, 2014, 2:32 dop., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119540/ --- (Updated Čec. 30, 2014, 2:32 dop.) 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119540: don't construct bogus KAuthAction objects
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119540/ --- (Updated Aug. 1, 2014, 2:35 p.m.) Status -- This change has been marked as submitted. 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119540: don't construct bogus KAuthAction objects
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119540/#review63509 --- 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) - Luca Beltrame On Lug. 30, 2014, 12:32 a.m., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119540/ --- (Updated Lug. 30, 2014, 12:32 a.m.) 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119540: don't construct bogus KAuthAction objects
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 request ed. 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
