Refactoring code is fine, however setting 644 unconditionally introduces a security issue. The permissions for new files should follow umask setting. Otherwise users may unintentionally share their new files with other users.

Since KIO::CopyJob::setDefaultPermissions does not work and all this KIO mambo jumbo is actually needed only for non-local FS, should we split this code and handle file creation for local and non-local files differently? I haven't checked yet, but I guess QFile should be able to handle umask correctly. If not, there are bunch of other ways to create a local file.

SQUASH_ME Panel: Create new local text files directly with QFile

Harbormaster completed remote builds in B9131: Diff 53075.Mar 3 2019, 7:05 PM

You're right Nikita. I changed it now so that local files are created with QFile. Umask is applied here.

Actually the code belongs into the FileSystem package, but thats a bigger change I'll eventually do later.

abika retitled this revision from Set permissions of new text files to 644 (-rw-r--r--) to Create new text files with umask applied.Mar 3 2019, 7:16 PM

abika edited the summary of this revision. (Show Details)

Alex, it looks good now - thanks! Let me test it and I'll get back to you in a few days.

Tested with various umasks and local / remote fs - works nicely. Please don't forget to fix the build (and possibly, the commit messages) before merging. Thanks!

krusader/Panel/panelfunc.cpp
515	code suggestion: return here and on line 524 could be combined into a single return inside the parent if
520	Doesn't build: missing semicolon.

This revision is now accepted and ready to land.Mar 7 2019, 6:53 AM

Thanks! Landed (Hope this revision will close itself).

krusader/Panel/panelfunc.cpp
515	I think its more clear with two returns.

Closed by commit R167:dce5c5057c85: Merge branch 'my-new-file-permissions' (authored by Alexander Bikadorov <alex.bikadorov@kdemail.net>). · Explain WhyMar 8 2019, 8:21 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			krusader/Panel/listpanel.h (2 lines)
M			krusader/Panel/listpanel.cpp (9 lines)
M			krusader/Panel/listpanelactions.h (4 lines)
M			krusader/Panel/listpanelactions.cpp (4 lines)
M			krusader/Panel/panelcontextmenu.h (2 lines)
M			krusader/Panel/panelcontextmenu.cpp (13 lines)
M			krusader/Panel/panelfunc.h (8 lines)
M			krusader/Panel/panelfunc.cpp (90 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	52448	65066df		Feb 24 2019, 4:01 PM	★	★
Diff 2	53075	65066df	- SQUASH_ME Panel: Create new local text files directly with QFile	Mar 3 2019, 7:05 PM	★	★
Diff 3	53475	a7ebda9	R167:dce5c5057c85a2d42e161f0dbd2dc6cd26c17a32	Mar 8 2019, 7:35 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
0b2aa42fd7c7	75077a69a195	8d60cd961dbc	Alexander Bikadorov	SQUASH_ME Panel: Create new local text files directly with QFile (Show More…)	Mar 3 2019, 7:02 PM
8d60cd961dbc	1ec869843966	b6ab9c38b710	Alexander Bikadorov	Set permissions of new text files to 644 (-rw-r--r--) (Show More…)	Feb 24 2019, 3:57 PM
b6ab9c38b710	f5767917798e	65066df6f57d	Alexander Bikadorov	Panel: Clean code for creating new text file (Show More…)	Feb 15 2019, 7:33 PM