Bug 342732 - FileCopyJob seems not to check whether target directory exists
Summary: FileCopyJob seems not to check whether target directory exists
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-11 14:51 UTC by Elias Probst
Modified: 2018-11-05 21:37 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Elias Probst 2015-01-11 14:51:04 UTC
When the directory ~/.cache/kioexec doesn't exist yet and "man:cp" is run from e.g. krunner, this error message will be shown.

Access denied.
Could not write to /home/elias/.cache/kioexec/krun/16719_0_cp.part.

This path is built in src/kioexec/main.cpp:110:
QString tmp = QStandardPaths::writableLocation(QStandardPaths::CacheLocation) + "/krun/"  +
              QString("%1_%2_%3").arg(QCoreApplication::applicationPid()).arg(jobCounter++).arg(fileName);

The actual job is created in 120:
KIO::Job *job = KIO::file_copy( url, dest );

Although the directory could be tested + created between 110 and 120, I feel like this should be done in a more generic way by KIO::file_copy() or even further up the job handling hierarchy.
Are there any explicit reasons not to ensure to heave created the target directory in the KIO::file_copy() job handling itself or is this just something which was simply missing until now?
Comment 1 David Faure 2015-02-01 18:28:22 UTC
It's not file_copy's job to create directories.

This is a porting bug, QStandardPaths doesn't create dirs while KStandardDirs did.

Thanks for spotting it. I'll commit the missing mkpath, can you re-test the bug and close it?
Comment 2 David Faure 2015-02-01 18:30:02 UTC
Git commit ff412e0bd3ba543bc1ad4b96a661c10844cd993d by David Faure.
Committed on 01/02/2015 at 18:28.
Pushed by dfaure into branch 'master'.

Fix porting error, mkpath needed after QStandardPaths::writableLocation().

Change-Id: Ib139c0f2af6538412a98b39cfb80fbb04c847705
CHANGELOG: fixed "Could not write to <path>" error when kioexec is triggered.

M  +2    -0    src/kioexec/main.cpp

http://commits.kde.org/kio/ff412e0bd3ba543bc1ad4b96a661c10844cd993d
Comment 3 Boris Egorov 2015-05-02 11:03:01 UTC
Git commit b290473167b9d0388715fffe494ee95a5c2c2851 by Boris Egorov.
Committed on 02/05/2015 at 10:47.
Pushed by egorov into branch 'master'.

Fix path for writable location for kurl (kioexec)

There was a porting issue with this code:
QStandardPaths::writableLocation do not creates a directory [1], so we
need QDir::mkpath. Commit ff412e0bd3ba54 tried to fix it, but it gone
too far: we need to create directory only up to "/krun/", strings after
that is filename. So we were creating directory instead of file,
confusing some apps. For example, kate [2].

New behavior was tested by removing CacheLocation, and it successfully
creates needed path.

    1: http://doc.qt.io/qt-5/qstandardpaths.html#writableLocation
    2: https://bugs.kde.org/show_bug.cgi?id=343329
Related: bug 343329
REVIEW: 123589
CHANGELOG: kioexec: Fixed path for writeable location for kurl

M  +3    -3    src/kioexec/main.cpp

http://commits.kde.org/kio/b290473167b9d0388715fffe494ee95a5c2c2851
Comment 4 Nate Graham 2018-08-22 19:42:20 UTC
Is there anything left to do here after those commits?
Comment 5 Nate Graham 2018-11-05 21:37:26 UTC
I'm guessing not. Closing until someone responds otherwise.