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>
This commit is contained in:
Berthold Stoeger 2018-05-16 00:17:04 +02:00 committed by Lubomir I. Ivanov
parent f4d569d89d
commit c3c2c2f606
2 changed files with 15 additions and 11 deletions

View file

@ -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;
}

View file

@ -46,7 +46,7 @@ signals:
void thumbnailChanged(QString filename, QImage thumbnail);
private:
Thumbnailer();
void processItem(QString filename);
void processItem(QString filename, bool tryDownload);
mutable QMutex lock;
QThreadPool pool;