Dive pictures: Don't enter infinity loop on invalid pictures

The recently committed refactoring of the dive-picture code introduced
a severe bug:

If an image couldn't be loaded from disk owing to an invalid file, the
filename was interpreted as an url and loaded in the background. This
succeeded, because the file actually exists. After download, the file
would then still be invalid and the whole thing restarted, leading to
an infinity loop.

To fix this, do two things:

1) Don't even try to download local files.
2) If interpreting a downloaded file fails, don't try the downloading
business again.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

1 files changed, 14 insertions, 10 deletions

diff --git a/core/imagedownloader.cpp b/core/imagedownloader.cpp
index 995bf2e02..372f5318a 100644
--- a/core/imagedownloader.cpp
+++ b/core/imagedownloader.cpp
@@ -39,8 +39,11 @@ void ImageDownloader::load(QString filename, bool fromHash)
 {
 	QUrl url = fromHash ? cloudImageURL(qPrintable(filename)) : QUrl::fromUserInput(filename);
 
-	if (!url.isValid())
+	// If this is a file, we tried previously -> don't bother trying it again
+	if (url.scheme() == "file" || !url.isValid()) {
 		emit failed(filename);
+		return;
+	}
 
 	QNetworkRequest request(url);
 	request.setAttribute(QNetworkRequest::User, filename);
@@ -99,14 +102,18 @@ static QImage loadImage(const QString &fileName, const char *format = nullptr)
 }
 
 // Returns: thumbnail, still loading
-static std::pair<QImage,bool> getHashedImage(const QString &file)
+static std::pair<QImage,bool> getHashedImage(const QString &file_in, bool tryDownload)
 {
+	QString file = file_in.startsWith("file://", Qt::CaseInsensitive) ? file_in.mid(7) : file_in;
 	QImage thumb;
 	bool stillLoading = false;
 	QUrl url = QUrl::fromUserInput(localFilePath(file));
 	if (url.isLocalFile())
 		thumb = loadImage(url.toLocalFile());
-	if (thumb.isNull()) {
+	if (!thumb.isNull()) {
+		// We loaded successfully. Now, make sure hash is up to date.
+		hashPicture(file);
+	} else if (tryDownload) {
 		// This did not load anything. Let's try to get the image from other sources
 		// Let's try to load it locally via its hash
 		QString filenameLocal = localFilePath(qPrintable(file));
@@ -129,9 +136,6 @@ static std::pair<QImage,bool> getHashedImage(const QString &file)
 				stillLoading = true;
 			}
 		}
-	} else {
-		// We loaded successfully. Now, make sure hash is up to date.
-		hashPicture(file);
 	}
 	return { thumb, stillLoading };
 }
@@ -208,12 +212,12 @@ static void addThumbnailToCache(const QImage &thumbnail, const QString &picture_
 	file.commit();
 }
 
-void Thumbnailer::processItem(QString filename)
+void Thumbnailer::processItem(QString filename, bool tryDownload)
 {
 	QImage thumbnail = getThumbnailFromCache(filename);
 
 	if (thumbnail.isNull()) {
-		auto res = getHashedImage(filename);
+		auto res = getHashedImage(filename, tryDownload);
 		if (res.second)
 			return;
 		thumbnail = res.first;
@@ -237,7 +241,7 @@ void Thumbnailer::imageDownloaded(QString filename)
 	// Image was downloaded and the filename connected with a hash.
 	// Try thumbnailing again.
 	QMutexLocker l(&lock);
-	workingOn[filename] = QtConcurrent::run(&pool, [this, filename]() { processItem(filename); });
+	workingOn[filename] = QtConcurrent::run(&pool, [this, filename]() { processItem(filename, false); });
 }
 
 void Thumbnailer::imageDownloadFailed(QString filename)
@@ -255,7 +259,7 @@ QImage Thumbnailer::fetchThumbnail(PictureEntry &entry)
 	const QString &filename = entry.filename;
 	if (!workingOn.contains(filename)) {
 		workingOn.insert(filename,
-				 QtConcurrent::run(&pool, [this, filename]() { processItem(filename); }));
+				 QtConcurrent::run(&pool, [this, filename]() { processItem(filename, true); }));
 	}
 	return dummyImage;
 }