D6833: Add support for PrivilegeExecution in KIO jobs
David Faure
noreply at phabricator.kde.org
Thu Jul 27 07:46:47 UTC 2017
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> chinmoyr wrote in copyjob.cpp:1674
> Ideally it should be set when m_privilegeExecutionEnabled is true. But it will add more lines. Even though its just 2-3 lines, it doesn't look good.
> Besides the flag is ineffective if the parent job doesn't have this flag set. Shall I remove it?
To me it looks worse to unconditionally pass a security-critical flag when it shouldn't be set, it reads like a security hole (even if, as you say, upon further research it turns out that the flag has no effect).
Maybe make it a helper method flagsForSubJob() to avoid any duplication.
BTW isn't the flag missing for the KIO::symlink case above?
> copyjob.cpp:294
> + break;
> + default:
> + break;
Remove default statement, which would leave the variable uninitialized. This way the compiler will warn if a new value is added to the CopyMode enum.
REVISION DETAIL
https://phabricator.kde.org/D6833
To: chinmoyr, dfaure, #frameworks
Cc: #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170727/a9422416/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list