Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129983/#review103158 --- ping apps 17.04 are out, this ca be useful - Anthony Fieroni On Март 27, 2017, 3 след обяд, Chinmoy Ranjan Pradhan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129983/ > --- > > (Updated Март 27, 2017, 3 след обяд) > > > Review request for KDE Frameworks, David Faure and Elvis Angelaccio. > > > Repository: kio > > > Description > --- > > This is regarding the GSOC idea > https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO. > > This patch intends to demonstrate one possible approach to provide polkit > support in kio. Here its only for the delete operation. This is based on the > patch in task https://phabricator.kde.org/T5070. > > The approach is as follows; > 1. Whenever file ioslave gets access denied error it calls the method > *execWithRoot* with the action that requires priviledge, the path of items >upon which action needs to be performed and a warning ID as arguments. > 2. *execWithRoot* then executes the KAuth::Action *org.kde.kio.file.execute*. > 3. This Kauth::Action has its Persistence set too 'session'. This means that > after authentication the restrictions are dropped for a while, for >about 5 minutes. This is similar to the behaviour of sudo command. > 4. During this time we can perform any action as a privileged user without > any authentication. So to prevent any mishap i added a warning box which >would popup before performing any action(only during this period). > 5. After the said time interval the root privileges are droped and calling > *execWithRoot* should show the usual authentication dialog. > > > Diffs > - > > src/ioslaves/file/CMakeLists.txt b9132ce > src/ioslaves/file/file.h 109ea80 > src/ioslaves/file/file.cpp eaf6c88 > src/ioslaves/file/file_unix.cpp 82eb11a > src/ioslaves/file/kauth/CMakeLists.txt PRE-CREATION > src/ioslaves/file/kauth/file.actions PRE-CREATION > src/ioslaves/file/kauth/helper.h PRE-CREATION > src/ioslaves/file/kauth/helper.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/129983/diff/ > > > Testing > --- > > > File Attachments > > > warning dialog > > https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png > > > Thanks, > > Chinmoy Ranjan Pradhan > >
D5394: KAuth integration in document saving - vol. 2
martinkostolny added a comment. One more thing I've changed in the last diff: When creating a new file in privileged directory, the file is then set to be readable by group and others. This is because kate wouldn't be able to read it later with regular user who managed to create it in the first place. I realize this is controversial so I will gladly remove this change if we agree on that. Probably the best approach would be to ask for permission to read it with another KAuth action, right? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5394 To: martinkostolny, #ktexteditor, fvogt Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni, cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, dfaure, #frameworks, head7, kfunk, sars
D5394: KAuth integration in document saving - vol. 2
martinkostolny updated this revision to Diff 14013.
martinkostolny added a comment.
Thanks for noticing the security issues! And sorry for the pause. Here is an
updated diff which should ensure, QFile is using relative path. I've managed to
reduce the use of absolute paths to this state - strace (saving privileged
example2.txt file inside ~/Downloads):
chdir("/home/kotelnik/Downloads") = 0
stat("example2.txt", {st_mode=S_IFREG|0640, st_size=1085659, ...}) = 0
getcwd("/home/kotelnik/Downloads", 4096) = 25
getpid()= 3343
open("/home/kotelnik/Downloads/example2.txt.TJ3343",
O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = 10
fcntl(10, F_SETFD, FD_CLOEXEC) = 0
lseek(10, 0, SEEK_SET) = 0
close(10) = 0
open("/tmp/kate.nS3280", O_RDONLY|O_CLOEXEC) = 10
fcntl(10, F_SETFD, FD_CLOEXEC) = 0
fstat(10, {st_mode=S_IFREG|0600, st_size=1085661, ...}) = 0
open("example2.txt.TJ3343", O_RDWR|O_CREAT|O_CLOEXEC, 0666) = 11
[...]
stat("example2.txt", {st_mode=S_IFREG|0640, st_size=1085659, ...}) = 0
access("example2.txt", R_OK)= 0
access("example2.txt", W_OK)= 0
access("example2.txt", X_OK)= -1 EACCES (Permission denied)
chmod("example2.txt.TJ3343", 0640) = 0
fchown(11, 33, 33) = 0
rename("example2.txt.TJ3343", "example2.txt") = 0
One not-nice part in code is opening and immediately closing the
QTemporaryFile (the only use of absolute path). Then tempFile is opened again
with relative path and written to like before. I wanted to make use of the
convenient way of creating unique temporary filename. Other suggestions are
welcome :).
Regarding parallel KAuth actions I didn't manage to understand KAuth code
enough to make a fix myself. So there comes 2 bug reports:
- https://bugs.kde.org/show_bug.cgi?id=379215
- https://bugs.kde.org/show_bug.cgi?id=379216
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5394?vs=13621=14013
REVISION DETAIL
https://phabricator.kde.org/D5394
AFFECTED FILES
src/buffer/katesecuretextbuffer.cpp
src/buffer/katesecuretextbuffer_p.h
src/buffer/katetextbuffer.cpp
src/buffer/katetextbuffer.h
To: martinkostolny, #ktexteditor, fvogt
Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni,
cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel,
dfaure, #frameworks, head7, kfunk, sars
D5646: Solve problem with microphone-sensitivity-medium
ltoscano added a comment. So this should rebased on top of the current master, I guess REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D5646 To: patrickelectric, #kate, andreaska Cc: ltoscano, winterz, #frameworks
D5656: Adds method to force the reloading of a document
pedroarthurp added reviewers: dfaure, brauch, dhaumann, cullmann. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5656 To: pedroarthurp, dfaure, brauch, dhaumann, cullmann Cc: kwrite-devel, #frameworks
D5646: Solve problem with microphone-sensitivity-medium
winterz added a comment. my commit was a hack to get the validator to pass. this patch is far more extensive. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D5646 To: patrickelectric, #kate, andreaska Cc: ltoscano, winterz, #frameworks
D5656: Adds method to force the reloading of a document
pedroarthurp created this revision. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY Currently, `KTextEditor::Document::documentReload()` shows a dialog asking the user if he/she wants to reload a file from disk. However, there are use cases where reloading the file from disk is the norm and no user interaction is required. For example, an external script would make changes in the file and those changes should be immediately reloaded. This patch then implements a mechanism through which `KTextEditor` clients could reload the file without asking user confirmation. The main motivation of this patch is to enable this no-confirmation-required file reloading to the external script facilities of kdevplataform. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5656 AFFECTED FILES src/document/katedocument.cpp src/document/katedocument.h src/include/ktexteditor/document.h To: pedroarthurp Cc: kwrite-devel, #frameworks
D5645: Add Arduino extension
This revision was automatically updated to reflect the committed changes. Closed by commit R216:ba8ed1f28885: Add Arduino extension (authored by patrickelectric). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5645?vs=13945=13985 REVISION DETAIL https://phabricator.kde.org/D5645 AFFECTED FILES data/syntax/cpp.xml To: patrickelectric, #kate, cullmann Cc: cullmann, #frameworks
D5645: Add Arduino extension
cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Strange endings but the internet tells me they are used for that. REPOSITORY R216 Syntax Highlighting BRANCH arduino REVISION DETAIL https://phabricator.kde.org/D5645 To: patrickelectric, #kate, cullmann Cc: cullmann, #frameworks
Re: Review Request 130090: Fix incorrect definition of major(3)/minor(3) macros
> On April 28, 2017, 1:28 p.m., Lamarque Souza wrote: > > Ship It! > > KJ Tsanaktsidis wrote: > Great - thanks for your help and for bearing with my rusty C++! What > happens now? I'm waiting on this patch to land for another patch I submitted > a while ago: https://git.reviewboard.kde.org/r/130084/ Well, if you do not have a developer account I can commit the patch for you. In order to do that I need your real name and email address to add them to the commit's header. - Lamarque --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130090/#review103148 --- On April 23, 2017, 9:56 a.m., KJ Tsanaktsidis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130090/ > --- > > (Updated April 23, 2017, 9:56 a.m.) > > > Review request for KDE Frameworks. > > > Repository: solid > > > Description > --- > > Previously, udesksblock.cpp was attempting to find a definition for > major/minor on Linux in by checking Q_OS_LINUX before > importing the header. Q_OS_LINUX is however only set when > qsystemdetection.h is included, and the macro was being checked first. > > Even had this check worked, it would still be wrong. On a modern version > of the userspace linux-headers, includes definitions for > major and minor that assume each is limited to 8 bits and that dev_t is > 16 bits. This is no longer true anymore; on Linux, major numbers can be > up to 12 bits at present and minor numbers up to 20. Calling these > macros with dev_t values > 2^16 would give incorrect results. > > Because the Q_OS_LINUX check failed, a fallback version of the macros > were defined for use on all platforms. The code is allegedly copied from > kdev_t.h, except it is copied from the *kernel* version of the header, > not the userspace version. Linux internally uses a different > representation of dev_t than it exposes to userspace - the kernelspace > version is 20 bits of minor/12 bits of major contiguously, but the > userspace version packs the bits in a different order to maintain > compatability with old 16-bit device numbers. Thus, this code also does > not work for dev_t values > 2^16. > > To fix this, we add CMake rules to search for a system-provided > definition of the major/minor macros - on various systems, these can be > in a few different places. As a fallback, we assume old-style 16-bit > dev_t (although I suspect that is only used for Windows, where > major/minor numbers are pretty meaningless anyway). > > > Diffs > - > > autotests/CMakeLists.txt 54adeea62b954b9169b37f1eab8fa3e215fafafa > autotests/fakeUdisks2.h PRE-CREATION > autotests/fakeUdisks2.cpp PRE-CREATION > autotests/solidudisks2test.cpp PRE-CREATION > src/solid/devices/backends/udisks2/CMakeLists.txt > 34390064af29ace07cbb3470945be098cc606d04 > src/solid/devices/backends/udisks2/udisksblock.cpp > 0622ec77fcf670a2005d34b7a6c31ca8b53a18d8 > > Diff: https://git.reviewboard.kde.org/r/130090/diff/ > > > Testing > --- > > I've written a little snippet to iterate through block devices, print their > major/minor number, and their device properties. It was previously > incorrectly labeling all my disks with major 0 and minor == device_number > (since it was using the first 20 bits for the minor). It now correctly > identifies their major/minor number. > > > Thanks, > > KJ Tsanaktsidis > >
D5646: Solve problem with microphone-sensitivity-medium
ltoscano added subscribers: winterz, ltoscano. ltoscano added a comment. @winterz committed https://phabricator.kde.org/R266:2b49fafc60235a83ffd8b92a24f800d622cc3050 REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D5646 To: patrickelectric, #kate, andreaska Cc: ltoscano, winterz, #frameworks
Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 561 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/561/ Project: PLATFORM=Linux,compiler=gcc Date of build: Sat, 29 Apr 2017 11:35:01 + Build duration: 14 min CHANGE SET Revision 2b49fafc60235a83ffd8b92a24f800d622cc3050 by Allen Winter: (icons/status/16/microphone-sensitivity-medium.svg - fix validation) change: edit icons/status/16/microphone-sensitivity-medium.svg JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 2/2 (100%)FILES 6/6 (100%)CLASSES 6/6 (100%)LINE 222/274 (81%)CONDITIONAL 125/218 (57%) By packages default> FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 14/26 (54%) autotests FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 180/217 (83%)CONDITIONAL 111/192 (58%)
Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 561 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/561/ Project: PLATFORM=Linux,compiler=gcc Date of build: Sat, 29 Apr 2017 11:35:01 + Build duration: 14 min CHANGE SET Revision 2b49fafc60235a83ffd8b92a24f800d622cc3050 by Allen Winter: (icons/status/16/microphone-sensitivity-medium.svg - fix validation) change: edit icons/status/16/microphone-sensitivity-medium.svg JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 2/2 (100%)FILES 6/6 (100%)CLASSES 6/6 (100%)LINE 222/274 (81%)CONDITIONAL 125/218 (57%) By packages default> FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 14/26 (54%) autotests FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 180/217 (83%)CONDITIONAL 111/192 (58%)
Breeze Icons Compilation Failure
Hi all, Is there some reason why the Breeze Icons repository was left broken for two days? Having a Frameworks level repository broken for two days (given we have monthly release windows) isn't exactly the best look. As a reminder to everyone, it is an obligation of contributors to only commit/push code which compiles as mentioned in the instructions sent to you by Sysadmin when your commit account was granted. Regards, Ben Cooksley KDE Sysadmin
Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 560 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/560/ Project: PLATFORM=Linux,compiler=gcc Date of build: Sat, 29 Apr 2017 11:35:01 + Build duration: 3 min 27 sec CHANGE SET Revision 2b49fafc60235a83ffd8b92a24f800d622cc3050 by Allen Winter: (icons/status/16/microphone-sensitivity-medium.svg - fix validation) change: edit icons/status/16/microphone-sensitivity-medium.svg JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 2/2 (100%)FILES 6/6 (100%)CLASSES 6/6 (100%)LINE 222/274 (81%)CONDITIONAL 125/218 (57%) By packages default> FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 14/26 (54%) autotests FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 180/217 (83%)CONDITIONAL 111/192 (58%)
Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 560 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/560/ Project: PLATFORM=Linux,compiler=gcc Date of build: Sat, 29 Apr 2017 11:35:01 + Build duration: 3 min 27 sec CHANGE SET Revision 2b49fafc60235a83ffd8b92a24f800d622cc3050 by Allen Winter: (icons/status/16/microphone-sensitivity-medium.svg - fix validation) change: edit icons/status/16/microphone-sensitivity-medium.svg JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 2/2 (100%)FILES 6/6 (100%)CLASSES 6/6 (100%)LINE 222/274 (81%)CONDITIONAL 125/218 (57%) By packages default> FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 14/26 (54%) autotests FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 180/217 (83%)CONDITIONAL 111/192 (58%)
Re: Review Request 130090: Fix incorrect definition of major(3)/minor(3) macros
> On April 28, 2017, 1:28 p.m., Lamarque Souza wrote: > > Ship It! Great - thanks for your help and for bearing with my rusty C++! What happens now? I'm waiting on this patch to land for another patch I submitted a while ago: https://git.reviewboard.kde.org/r/130084/ - KJ --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130090/#review103148 --- On April 23, 2017, 9:56 a.m., KJ Tsanaktsidis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130090/ > --- > > (Updated April 23, 2017, 9:56 a.m.) > > > Review request for KDE Frameworks. > > > Repository: solid > > > Description > --- > > Previously, udesksblock.cpp was attempting to find a definition for > major/minor on Linux in by checking Q_OS_LINUX before > importing the header. Q_OS_LINUX is however only set when > qsystemdetection.h is included, and the macro was being checked first. > > Even had this check worked, it would still be wrong. On a modern version > of the userspace linux-headers, includes definitions for > major and minor that assume each is limited to 8 bits and that dev_t is > 16 bits. This is no longer true anymore; on Linux, major numbers can be > up to 12 bits at present and minor numbers up to 20. Calling these > macros with dev_t values > 2^16 would give incorrect results. > > Because the Q_OS_LINUX check failed, a fallback version of the macros > were defined for use on all platforms. The code is allegedly copied from > kdev_t.h, except it is copied from the *kernel* version of the header, > not the userspace version. Linux internally uses a different > representation of dev_t than it exposes to userspace - the kernelspace > version is 20 bits of minor/12 bits of major contiguously, but the > userspace version packs the bits in a different order to maintain > compatability with old 16-bit device numbers. Thus, this code also does > not work for dev_t values > 2^16. > > To fix this, we add CMake rules to search for a system-provided > definition of the major/minor macros - on various systems, these can be > in a few different places. As a fallback, we assume old-style 16-bit > dev_t (although I suspect that is only used for Windows, where > major/minor numbers are pretty meaningless anyway). > > > Diffs > - > > autotests/CMakeLists.txt 54adeea62b954b9169b37f1eab8fa3e215fafafa > autotests/fakeUdisks2.h PRE-CREATION > autotests/fakeUdisks2.cpp PRE-CREATION > autotests/solidudisks2test.cpp PRE-CREATION > src/solid/devices/backends/udisks2/CMakeLists.txt > 34390064af29ace07cbb3470945be098cc606d04 > src/solid/devices/backends/udisks2/udisksblock.cpp > 0622ec77fcf670a2005d34b7a6c31ca8b53a18d8 > > Diff: https://git.reviewboard.kde.org/r/130090/diff/ > > > Testing > --- > > I've written a little snippet to iterate through block devices, print their > major/minor number, and their device properties. It was previously > incorrectly labeling all my disks with major 0 and minor == device_number > (since it was using the first 20 bits for the minor). It now correctly > identifies their major/minor number. > > > Thanks, > > KJ Tsanaktsidis > >
D5631: Fix directory based search.
lbeltrame accepted this revision. lbeltrame added a comment. This revision is now accepted and ready to land. Looks fairly harmless to me. Perhaps wait one or two days more to see if anyone chimes in. REPOSITORY R293 Baloo BRANCH master REVISION DETAIL https://phabricator.kde.org/D5631 To: xuetianweng, #frameworks, lbeltrame Cc: lbeltrame
D5646: Solve problem with microphone-sensitivity-medium
lbeltrame added a reviewer: andreaska. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D5646 To: patrickelectric, #kate, andreaska Cc: #frameworks
