From 649f78d0a4475a640ad353e1e879a7bb27db222b Mon Sep 17 00:00:00 2001 From: Liav A Date: Sun, 15 Jan 2023 21:50:32 +0200 Subject: [PATCH] LibGfx+Ladybird+Userland: Don't sniff for TGA images with only raw bytes Because TGA images don't have magic bytes as a signature to be detected, instead assume a sequence of ReadonlyBytes is a possible TGA image only if we are given a path so we could check the extension of the file and see if it's a TGA image. When we know the path of the file being loaded, we will try to first check its extension, and only if there's no match to a known decoder, based on simple extension lookup, then we would probe for other formats as usual with the normal sniffing method. --- Ladybird/ImageCodecPluginLadybird.cpp | 2 +- .../Applications/ImageViewer/ViewWidget.cpp | 2 +- Userland/Libraries/LibGUI/ImageWidget.cpp | 2 +- Userland/Libraries/LibGfx/Bitmap.cpp | 2 +- Userland/Libraries/LibGfx/ImageDecoder.cpp | 94 +++++++++++-------- Userland/Libraries/LibGfx/ImageDecoder.h | 3 +- .../LibImageDecoderClient/Client.cpp | 37 ++++++++ .../Libraries/LibImageDecoderClient/Client.h | 1 + .../ImageDecoder/ConnectionFromClient.cpp | 80 +++++++++++----- .../ImageDecoder/ConnectionFromClient.h | 1 + .../ImageDecoder/ImageDecoderServer.ipc | 1 + Userland/Utilities/file.cpp | 2 +- Userland/Utilities/headless-browser.cpp | 2 +- 13 files changed, 162 insertions(+), 67 deletions(-) diff --git a/Ladybird/ImageCodecPluginLadybird.cpp b/Ladybird/ImageCodecPluginLadybird.cpp index c00c10ac43..b11215cf96 100644 --- a/Ladybird/ImageCodecPluginLadybird.cpp +++ b/Ladybird/ImageCodecPluginLadybird.cpp @@ -40,7 +40,7 @@ static Optional decode_image_with_qt(ReadonlyBytes static Optional decode_image_with_libgfx(ReadonlyBytes data) { - auto decoder = Gfx::ImageDecoder::try_create(data); + auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(data); if (!decoder || !decoder->frame_count()) { return {}; diff --git a/Userland/Applications/ImageViewer/ViewWidget.cpp b/Userland/Applications/ImageViewer/ViewWidget.cpp index 258b053d10..44c4ccb774 100644 --- a/Userland/Applications/ImageViewer/ViewWidget.cpp +++ b/Userland/Applications/ImageViewer/ViewWidget.cpp @@ -168,7 +168,7 @@ void ViewWidget::load_from_file(DeprecatedString const& path) // Spawn a new ImageDecoder service process and connect to it. auto client = ImageDecoderClient::Client::try_create().release_value_but_fixme_should_propagate_errors(); - auto decoded_image_or_error = client->decode_image(mapped_file.bytes()); + auto decoded_image_or_error = client->decode_image_with_known_path(path, mapped_file.bytes()); if (!decoded_image_or_error.has_value()) { show_error(); return; diff --git a/Userland/Libraries/LibGUI/ImageWidget.cpp b/Userland/Libraries/LibGUI/ImageWidget.cpp index 27c42b407a..2800fe4f75 100644 --- a/Userland/Libraries/LibGUI/ImageWidget.cpp +++ b/Userland/Libraries/LibGUI/ImageWidget.cpp @@ -76,7 +76,7 @@ void ImageWidget::load_from_file(StringView path) return; auto& mapped_file = *file_or_error.value(); - m_image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes()); + m_image_decoder = Gfx::ImageDecoder::try_create_for_raw_bytes_with_known_path(path, mapped_file.bytes()); VERIFY(m_image_decoder); auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors(); diff --git a/Userland/Libraries/LibGfx/Bitmap.cpp b/Userland/Libraries/LibGfx/Bitmap.cpp index 4f15f04a42..30073c6ae0 100644 --- a/Userland/Libraries/LibGfx/Bitmap.cpp +++ b/Userland/Libraries/LibGfx/Bitmap.cpp @@ -143,7 +143,7 @@ ErrorOr> Bitmap::try_load_from_file(StringView path, int s ErrorOr> Bitmap::try_load_from_fd_and_close(int fd, StringView path) { auto file = TRY(Core::MappedFile::map_from_fd_and_close(fd, path)); - if (auto decoder = ImageDecoder::try_create(file->bytes())) { + if (auto decoder = ImageDecoder::try_create_for_raw_bytes_with_known_path(path, file->bytes())) { auto frame = TRY(decoder->frame(0)); if (auto& bitmap = frame.image) return bitmap.release_nonnull(); diff --git a/Userland/Libraries/LibGfx/ImageDecoder.cpp b/Userland/Libraries/LibGfx/ImageDecoder.cpp index 76093068a3..9074172959 100644 --- a/Userland/Libraries/LibGfx/ImageDecoder.cpp +++ b/Userland/Libraries/LibGfx/ImageDecoder.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -19,63 +20,82 @@ namespace Gfx { -RefPtr ImageDecoder::try_create(ReadonlyBytes bytes) +static OwnPtr probe_and_sniff_for_appropriate_plugin(ReadonlyBytes bytes) { auto* data = bytes.data(); auto size = bytes.size(); + OwnPtr plugin; - auto plugin = [](auto* data, auto size) -> OwnPtr { - OwnPtr plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + plugin = make(data, size); + if (plugin->sniff()) + return plugin; - plugin = make(data, size); - if (plugin->sniff()) - return plugin; + return {}; +} +RefPtr ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes bytes) +{ + OwnPtr plugin = probe_and_sniff_for_appropriate_plugin(bytes); + if (!plugin) + return {}; + return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull())); +} + +static OwnPtr probe_and_sniff_for_appropriate_plugin_with_known_path(StringView path, ReadonlyBytes bytes) +{ + LexicalPath lexical_mapped_file_path(path); + auto* data = bytes.data(); + auto size = bytes.size(); + OwnPtr plugin; + if (lexical_mapped_file_path.extension() == "tga"sv) { plugin = make(data, size); if (plugin->sniff()) return plugin; + } + return {}; +} - return {}; - }(data, size); - +RefPtr ImageDecoder::try_create_for_raw_bytes_with_known_path(StringView path, ReadonlyBytes bytes) +{ + OwnPtr plugin = probe_and_sniff_for_appropriate_plugin_with_known_path(path, bytes); if (!plugin) - return {}; + return try_create_for_raw_bytes(bytes); return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull())); } diff --git a/Userland/Libraries/LibGfx/ImageDecoder.h b/Userland/Libraries/LibGfx/ImageDecoder.h index 431a26fd7e..5b0731b9a2 100644 --- a/Userland/Libraries/LibGfx/ImageDecoder.h +++ b/Userland/Libraries/LibGfx/ImageDecoder.h @@ -47,7 +47,8 @@ protected: class ImageDecoder : public RefCounted { public: - static RefPtr try_create(ReadonlyBytes); + static RefPtr try_create_for_raw_bytes(ReadonlyBytes); + static RefPtr try_create_for_raw_bytes_with_known_path(StringView path, ReadonlyBytes); ~ImageDecoder() = default; IntSize size() const { return m_plugin->size(); } diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp index 766902bf20..abb151c279 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.cpp +++ b/Userland/Libraries/LibImageDecoderClient/Client.cpp @@ -20,6 +20,43 @@ void Client::die() on_death(); } +Optional Client::decode_image_with_known_path(DeprecatedString const& path, ReadonlyBytes encoded_data) +{ + if (encoded_data.is_empty()) + return {}; + + auto encoded_buffer_or_error = Core::AnonymousBuffer::create_with_size(encoded_data.size()); + if (encoded_buffer_or_error.is_error()) { + dbgln("Could not allocate encoded buffer"); + return {}; + } + auto encoded_buffer = encoded_buffer_or_error.release_value(); + + memcpy(encoded_buffer.data(), encoded_data.data(), encoded_data.size()); + auto response_or_error = try_decode_image_with_known_path(path, move(encoded_buffer)); + + if (response_or_error.is_error()) { + dbgln("ImageDecoder died heroically"); + return {}; + } + + auto& response = response_or_error.value(); + + if (response.bitmaps().is_empty()) + return {}; + + DecodedImage image; + image.is_animated = response.is_animated(); + image.loop_count = response.loop_count(); + image.frames.resize(response.bitmaps().size()); + for (size_t i = 0; i < image.frames.size(); ++i) { + auto& frame = image.frames[i]; + frame.bitmap = response.bitmaps()[i].bitmap(); + frame.duration = response.durations()[i]; + } + return image; +} + Optional Client::decode_image(ReadonlyBytes encoded_data) { if (encoded_data.is_empty()) diff --git a/Userland/Libraries/LibImageDecoderClient/Client.h b/Userland/Libraries/LibImageDecoderClient/Client.h index 02fa7f8147..4e15e9ea2e 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.h +++ b/Userland/Libraries/LibImageDecoderClient/Client.h @@ -31,6 +31,7 @@ class Client final public: Optional decode_image(ReadonlyBytes); + Optional decode_image_with_known_path(DeprecatedString const& path, ReadonlyBytes); Function on_death; diff --git a/Userland/Services/ImageDecoder/ConnectionFromClient.cpp b/Userland/Services/ImageDecoder/ConnectionFromClient.cpp index e95f9ae066..9f0bee865b 100644 --- a/Userland/Services/ImageDecoder/ConnectionFromClient.cpp +++ b/Userland/Services/ImageDecoder/ConnectionFromClient.cpp @@ -22,29 +22,10 @@ void ConnectionFromClient::die() Core::EventLoop::current().quit(0); } -Messages::ImageDecoderServer::DecodeImageResponse ConnectionFromClient::decode_image(Core::AnonymousBuffer const& encoded_buffer) +static void decode_image_to_bitmaps_and_durations_with_decoder(Gfx::ImageDecoder const& decoder, Vector& bitmaps, Vector& durations) { - if (!encoded_buffer.is_valid()) { - dbgln_if(IMAGE_DECODER_DEBUG, "Encoded data is invalid"); - return nullptr; - } - - auto decoder = Gfx::ImageDecoder::try_create(ReadonlyBytes { encoded_buffer.data(), encoded_buffer.size() }); - - if (!decoder) { - dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data"); - return { false, 0, Vector {}, Vector {} }; - } - - if (!decoder->frame_count()) { - dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data"); - return { false, 0, Vector {}, Vector {} }; - } - - Vector bitmaps; - Vector durations; - for (size_t i = 0; i < decoder->frame_count(); ++i) { - auto frame_or_error = decoder->frame(i); + for (size_t i = 0; i < decoder.frame_count(); ++i) { + auto frame_or_error = decoder.frame(i); if (frame_or_error.is_error()) { bitmaps.append(Gfx::ShareableBitmap {}); durations.append(0); @@ -54,8 +35,61 @@ Messages::ImageDecoderServer::DecodeImageResponse ConnectionFromClient::decode_i durations.append(frame.duration); } } +} - return { decoder->is_animated(), static_cast(decoder->loop_count()), bitmaps, durations }; +static void decode_image_to_details(Core::AnonymousBuffer const& encoded_buffer, Optional known_path, bool& is_animated, u32& loop_count, Vector& bitmaps, Vector& durations) +{ + VERIFY(bitmaps.size() == 0); + VERIFY(durations.size() == 0); + VERIFY(!is_animated); + VERIFY(loop_count == 0); + + RefPtr decoder; + if (known_path.has_value()) + decoder = Gfx::ImageDecoder::try_create_for_raw_bytes_with_known_path(known_path.value(), ReadonlyBytes { encoded_buffer.data(), encoded_buffer.size() }); + else + decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes { encoded_buffer.data(), encoded_buffer.size() }); + + if (!decoder) { + dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data"); + return; + } + + if (!decoder->frame_count()) { + dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data"); + return; + } + decode_image_to_bitmaps_and_durations_with_decoder(*decoder, bitmaps, durations); +} + +Messages::ImageDecoderServer::DecodeImageWithKnownPathResponse ConnectionFromClient::decode_image_with_known_path(DeprecatedString const& path, Core::AnonymousBuffer const& encoded_buffer) +{ + if (!encoded_buffer.is_valid()) { + dbgln_if(IMAGE_DECODER_DEBUG, "Encoded data is invalid"); + return nullptr; + } + + bool is_animated = false; + u32 loop_count = 0; + Vector bitmaps; + Vector durations; + decode_image_to_details(encoded_buffer, path.substring_view(0), is_animated, loop_count, bitmaps, durations); + return { is_animated, loop_count, bitmaps, durations }; +} + +Messages::ImageDecoderServer::DecodeImageResponse ConnectionFromClient::decode_image(Core::AnonymousBuffer const& encoded_buffer) +{ + if (!encoded_buffer.is_valid()) { + dbgln_if(IMAGE_DECODER_DEBUG, "Encoded data is invalid"); + return nullptr; + } + + bool is_animated = false; + u32 loop_count = 0; + Vector bitmaps; + Vector durations; + decode_image_to_details(encoded_buffer, {}, is_animated, loop_count, bitmaps, durations); + return { is_animated, loop_count, bitmaps, durations }; } } diff --git a/Userland/Services/ImageDecoder/ConnectionFromClient.h b/Userland/Services/ImageDecoder/ConnectionFromClient.h index 6b8a5e8086..f137af3dfe 100644 --- a/Userland/Services/ImageDecoder/ConnectionFromClient.h +++ b/Userland/Services/ImageDecoder/ConnectionFromClient.h @@ -27,6 +27,7 @@ private: explicit ConnectionFromClient(NonnullOwnPtr); virtual Messages::ImageDecoderServer::DecodeImageResponse decode_image(Core::AnonymousBuffer const&) override; + virtual Messages::ImageDecoderServer::DecodeImageWithKnownPathResponse decode_image_with_known_path(DeprecatedString const& path, Core::AnonymousBuffer const&) override; }; } diff --git a/Userland/Services/ImageDecoder/ImageDecoderServer.ipc b/Userland/Services/ImageDecoder/ImageDecoderServer.ipc index 5ca2e40ea0..9492f685a0 100644 --- a/Userland/Services/ImageDecoder/ImageDecoderServer.ipc +++ b/Userland/Services/ImageDecoder/ImageDecoderServer.ipc @@ -4,4 +4,5 @@ endpoint ImageDecoderServer { decode_image(Core::AnonymousBuffer data) => (bool is_animated, u32 loop_count, Vector bitmaps, Vector durations) + decode_image_with_known_path(DeprecatedString path, Core::AnonymousBuffer data) => (bool is_animated, u32 loop_count, Vector bitmaps, Vector durations) } diff --git a/Userland/Utilities/file.cpp b/Userland/Utilities/file.cpp index e22855f95b..8c4f0dd68f 100644 --- a/Userland/Utilities/file.cpp +++ b/Userland/Utilities/file.cpp @@ -33,7 +33,7 @@ static Optional image_details(DeprecatedString const& descript return {}; auto& mapped_file = *file_or_error.value(); - auto image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes()); + auto image_decoder = Gfx::ImageDecoder::try_create_for_raw_bytes_with_known_path(path, mapped_file.bytes()); if (!image_decoder) return {}; diff --git a/Userland/Utilities/headless-browser.cpp b/Userland/Utilities/headless-browser.cpp index bc5705aeb9..361413440e 100644 --- a/Userland/Utilities/headless-browser.cpp +++ b/Userland/Utilities/headless-browser.cpp @@ -265,7 +265,7 @@ public: virtual Optional decode_image(ReadonlyBytes data) override { - auto decoder = Gfx::ImageDecoder::try_create(data); + auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(data); if (!decoder) return Web::Platform::DecodedImage { false, 0, Vector {} };