----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129844/#review102070 -----------------------------------------------------------
Ship it! 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. - David Faure 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 > >
