From 66910507dc5cb2e51a80c90ad7d8c31b4e7eeb67 Mon Sep 17 00:00:00 2001 From: hemirt <1310440+hemirt@users.noreply.github.com> Date: Sat, 24 Feb 2024 13:52:35 +0100 Subject: [PATCH] Fix incomplete traversal of clipboard data when an image is present resulting in Not an Image error (#5156) --- CHANGELOG.md | 1 + src/singletons/ImageUploader.cpp | 90 +++++++++++++++++--------------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc0f9369..0f6c026e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ - Bugfix: Reply contexts now use the color of the replied-to message. (#5145) - Bugfix: Fixed top-level window getting stuck after opening settings. (#5161, #5166) - Bugfix: Fixed link info not updating without moving the cursor. (#5178) +- Bugfix: Fixed an upload sometimes failing when copying an image from a browser if it contained extra properties. (#5156) - Bugfix: Fixed tooltips getting out of bounds when loading images. (#5186) - Dev: Run miniaudio in a separate thread, and simplify it to not manage the device ourselves. There's a chance the simplification is a bad idea. (#4978) - Dev: Change clang-format from v14 to v16. (#4929) diff --git a/src/singletons/ImageUploader.cpp b/src/singletons/ImageUploader.cpp index d528c9929..f8ad53d2e 100644 --- a/src/singletons/ImageUploader.cpp +++ b/src/singletons/ImageUploader.cpp @@ -238,7 +238,7 @@ void ImageUploader::handleSuccessfulUpload(const NetworkResult &result, } else { - QTimer::singleShot(UPLOAD_DELAY, [channel, &textEdit, this]() { + QTimer::singleShot(UPLOAD_DELAY, [channel, textEdit, this]() { this->sendImageUploadRequest(this->uploadQueue_.front(), channel, textEdit); this->uploadQueue_.pop(); @@ -259,8 +259,11 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel, } channel->addMessage(makeSystemMessage(QString("Started upload..."))); - if (source->hasUrls()) - { + auto tryUploadFromUrls = [&]() -> bool { + if (!source->hasUrls()) + { + return false; + } auto mimeDb = QMimeDatabase(); // This path gets chosen when files are copied from a file manager, like explorer.exe, caja. // Each entry in source->urls() is a QUrl pointing to a file that was copied. @@ -277,8 +280,7 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel, { channel->addMessage( makeSystemMessage(QString("Couldn't load image :("))); - this->uploadMutex_.unlock(); - return; + return false; } auto imageData = convertToPng(img); @@ -293,8 +295,7 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel, QString("Cannot upload file: %1. Couldn't convert " "image to png.") .arg(localPath))); - this->uploadMutex_.unlock(); - return; + return false; } } else if (mime.inherits("image/gif")) @@ -307,21 +308,12 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel, { channel->addMessage( makeSystemMessage(QString("Failed to open file. :("))); - this->uploadMutex_.unlock(); - return; + return false; } + // file.readAll() => might be a bit big but it /should/ work RawImageData data = {file.readAll(), "gif", localPath}; this->uploadQueue_.push(data); file.close(); - // file.readAll() => might be a bit big but it /should/ work - } - else - { - channel->addMessage(makeSystemMessage( - QString("Cannot upload file: %1. Not an image.") - .arg(localPath))); - this->uploadMutex_.unlock(); - return; } } if (!this->uploadQueue_.empty()) @@ -329,40 +321,52 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel, this->sendImageUploadRequest(this->uploadQueue_.front(), channel, outputTextEdit); this->uploadQueue_.pop(); + return true; } - } - else if (source->hasFormat("image/png")) - { - // the path to file is not present every time, thus the filePath is empty - this->sendImageUploadRequest({source->data("image/png"), "png", ""}, - channel, outputTextEdit); - } - else if (source->hasFormat("image/jpeg")) - { - this->sendImageUploadRequest({source->data("image/jpeg"), "jpeg", ""}, - channel, outputTextEdit); - } - else if (source->hasFormat("image/gif")) - { - this->sendImageUploadRequest({source->data("image/gif"), "gif", ""}, - channel, outputTextEdit); - } + return false; + }; - else - { // not PNG, try loading it into QImage and save it to a PNG. + auto tryUploadDirectly = [&]() -> bool { + if (source->hasFormat("image/png")) + { + // the path to file is not present every time, thus the filePath is empty + this->sendImageUploadRequest({source->data("image/png"), "png", ""}, + channel, outputTextEdit); + return true; + } + if (source->hasFormat("image/jpeg")) + { + this->sendImageUploadRequest( + {source->data("image/jpeg"), "jpeg", ""}, channel, + outputTextEdit); + return true; + } + if (source->hasFormat("image/gif")) + { + this->sendImageUploadRequest({source->data("image/gif"), "gif", ""}, + channel, outputTextEdit); + return true; + } + // not PNG, try loading it into QImage and save it to a PNG. auto image = qvariant_cast(source->imageData()); auto imageData = convertToPng(image); if (imageData) { sendImageUploadRequest({*imageData, "png", ""}, channel, outputTextEdit); + return true; } - else - { - channel->addMessage(makeSystemMessage( - QString("Cannot upload file, failed to convert to png."))); - this->uploadMutex_.unlock(); - } + // No direct upload happenned + channel->addMessage(makeSystemMessage( + QString("Cannot upload file, failed to convert to png."))); + return false; + }; + + if (!tryUploadFromUrls() && !tryUploadDirectly()) + { + channel->addMessage( + makeSystemMessage(QString("Cannot upload file from clipboard."))); + this->uploadMutex_.unlock(); } }