> 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.
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? - Elvis ----------------------------------------------------------- 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 > >