Review Request 129844: KRun: deprecate runUrl() in favor of runUrl() with RunFlags

[David Faure]

> On Jan. 17, 2017, 7:51 a.m., David Faure wrote:
> > enums instead of bools in APIs are good.
> > 
> > It's funny to read "I want to change the default value [....] but the new method can't have a default value", but it actually makes sense because the old method is deprecated, so anyone porting away from it will have to set the flags.
> > 
> > BTW the default was true because the original use case was filemanagers (and krunner), which do want to run executables. But indeed in most apps it's probably not a good idea.
> 
> Elvis Angelaccio wrote:
>     I wanted to ask about the `asn` argument but then I forgot to.
>     There is a comment `// TODO KF6: deprecate/remove` near the asn argument of runService(). Does that apply also to runUrl()? If yes, should we remove it now?

That comment is about all of the runService method, not about the asn argument itself, see dc601bd.
Don't remove asn, it's needed.


- David


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


On Jan. 15, 2017, 8:46 p.m., Elvis Angelaccio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129844/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2017, 8:46 p.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> CVE-2017-5330 shows that `runExecutables = true` can be a dangerous
> default for the runUrl() function. We cannot change the default value to
> false (while BIC, it would be a change of behavior), so we deprecate the
> current runUrl() function in favor of a new runUrl() with a RunFlags
> argument replacing the `tempFile` and `runExecutables` arguments.
> 
> This new argument cannot take a default value, otherwise the two
> runUrl() signatures would be ambiguous and existing code
> would not compile.
> 
> 
> Diffs
> -----
> 
>   src/widgets/krun.h 889642160ad960dd7e43d1c6dad2a6f2133e17bf 
>   src/widgets/krun.cpp d04a4825e5ea696c1072054c39dc11cc9e5c63f5 
> 
> Diff: https://git.reviewboard.kde.org/r/129844/diff/
> 
> 
> Testing
> -------
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170117/3ba37464/attachment-0001.html>