Differential D7146 Diff 17762 plugins/notifications/notification.cpp

Changeset View

Standalone View

View Options

plugins/notifications/notification.cpp

Context not available.
26#include <QUrl> 26​#include <QUrl>
27#include <QPixmap> 27​#include <QPixmap>
28#include <KLocalizedString> 28​#include <KLocalizedString>
29​#include <QStandardPaths>
29 30​
30#include <core/filetransferjob.h> 31​#include <core/filetransferjob.h>
31 32​
Context not available.
33Notification::Notification(const NetworkPackage& np, QObject* parent) 34​Notification::Notification(const NetworkPackage& np, QObject* parent)
34 : QObject(parent) 35​ : QObject(parent)
35{ 36​{
apolUnsubmitted
Not Done

Use QStandardPaths, this isn't portable.

apol: Use `QStandardPaths`, this isn't portable.
36 mImagesDir = QDir::temp().absoluteFilePath(QStringLiteral("kdeconnect")); 37​ // Use CacheLocation instead of TempLocation because TempLocation may be accessible system-wide
thomaspUnsubmitted
Not Done

Are the icons reused over restarts? In that case a cache is what you want.
If they are only short lived, TempLocation is a better fit as /tmp is usually a tmpfs(5).
Writing the files to disk just to delete them 5 seconds later does nothing but wear the disk.
To address the privacy concerns, set appropriate permissions on the directory (i.e. only owner should have access)

thomasp: Are the icons reused over restarts? In that case a cache is what you want. If they are only…
38​ mImagesDir = QDir(QStandardPaths::writableLocation(QStandardPaths::CacheLocation));
37 mImagesDir.mkpath(mImagesDir.absolutePath()); 39​ mImagesDir.mkpath(mImagesDir.absolutePath());
38 mClosed = false; 40​ mClosed = false;
39 41​
thomaspUnsubmitted
Not Done

The file is created and filled with a FileTransferJob. Setting the file permission after the job has finished does not help privacy. Please set the permissions on the directory.

thomasp: The file is created and filled with a FileTransferJob. Setting the file permission after the…
Context not available.
113 } else { 115​ } else {
114 mIconPath = mImagesDir.absoluteFilePath(filename); 116​ mIconPath = mImagesDir.absoluteFilePath(filename);
115 QUrl destinationUrl(mIconPath); 117​ QUrl destinationUrl(mIconPath);
apolUnsubmitted
Not Done

QUrl destinationUrl = QUrl::fromLocalPath(mIconPath);

And drop the line below

apol: `QUrl destinationUrl = QUrl::fromLocalPath(mIconPath);` And drop the line below
118​ destinationUrl.setScheme(QStringLiteral("file"));
116 FileTransferJob* job = np.createPayloadTransferJob(destinationUrl); 119​ FileTransferJob* job = np.createPayloadTransferJob(destinationUrl);
thomaspUnsubmitted
Not Done

Where is this file deleted?

thomasp: Where is this file deleted?
117 job->start(); 120​ job->start();
118 connect(job, &FileTransferJob::result, this, &Notification::applyIconAndShow); 121​ connect(job, &FileTransferJob::result, this, &Notification::applyIconAndShow);
Context not available.