Parcourir la source

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.
Liav A il y a 2 ans
Parent
commit
649f78d0a4

+ 1 - 1
Ladybird/ImageCodecPluginLadybird.cpp

@@ -40,7 +40,7 @@ static Optional<Web::Platform::DecodedImage> decode_image_with_qt(ReadonlyBytes
 
 
 static Optional<Web::Platform::DecodedImage> decode_image_with_libgfx(ReadonlyBytes data)
 static Optional<Web::Platform::DecodedImage> 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()) {
     if (!decoder || !decoder->frame_count()) {
         return {};
         return {};

+ 1 - 1
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.
     // Spawn a new ImageDecoder service process and connect to it.
     auto client = ImageDecoderClient::Client::try_create().release_value_but_fixme_should_propagate_errors();
     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()) {
     if (!decoded_image_or_error.has_value()) {
         show_error();
         show_error();
         return;
         return;

+ 1 - 1
Userland/Libraries/LibGUI/ImageWidget.cpp

@@ -76,7 +76,7 @@ void ImageWidget::load_from_file(StringView path)
         return;
         return;
 
 
     auto& mapped_file = *file_or_error.value();
     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);
     VERIFY(m_image_decoder);
 
 
     auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();
     auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();

+ 1 - 1
Userland/Libraries/LibGfx/Bitmap.cpp

@@ -143,7 +143,7 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_file(StringView path, int s
 ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, StringView path)
 ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, StringView path)
 {
 {
     auto file = TRY(Core::MappedFile::map_from_fd_and_close(fd, 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));
         auto frame = TRY(decoder->frame(0));
         if (auto& bitmap = frame.image)
         if (auto& bitmap = frame.image)
             return bitmap.release_nonnull();
             return bitmap.release_nonnull();

+ 57 - 37
Userland/Libraries/LibGfx/ImageDecoder.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  * SPDX-License-Identifier: BSD-2-Clause
  */
  */
 
 
+#include <AK/LexicalPath.h>
 #include <LibGfx/BMPLoader.h>
 #include <LibGfx/BMPLoader.h>
 #include <LibGfx/DDSLoader.h>
 #include <LibGfx/DDSLoader.h>
 #include <LibGfx/GIFLoader.h>
 #include <LibGfx/GIFLoader.h>
@@ -19,63 +20,82 @@
 
 
 namespace Gfx {
 namespace Gfx {
 
 
-RefPtr<ImageDecoder> ImageDecoder::try_create(ReadonlyBytes bytes)
+static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin(ReadonlyBytes bytes)
 {
 {
     auto* data = bytes.data();
     auto* data = bytes.data();
     auto size = bytes.size();
     auto size = bytes.size();
+    OwnPtr<ImageDecoderPlugin> plugin;
 
 
-    auto plugin = [](auto* data, auto size) -> OwnPtr<ImageDecoderPlugin> {
-        OwnPtr<ImageDecoderPlugin> plugin;
+    plugin = make<PNGImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<PNGImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<GIFImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<GIFImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<BMPImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<BMPImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<PBMImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<PBMImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<PGMImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<PGMImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<PPMImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<PPMImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<ICOImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<ICOImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<JPGImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<JPGImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<DDSImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<DDSImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    plugin = make<QOIImageDecoderPlugin>(data, size);
+    if (plugin->sniff())
+        return plugin;
 
 
-        plugin = make<QOIImageDecoderPlugin>(data, size);
-        if (plugin->sniff())
-            return plugin;
+    return {};
+}
+
+RefPtr<ImageDecoder> ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes bytes)
+{
+    OwnPtr<ImageDecoderPlugin> plugin = probe_and_sniff_for_appropriate_plugin(bytes);
+    if (!plugin)
+        return {};
+    return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
+}
 
 
+static OwnPtr<ImageDecoderPlugin> 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<ImageDecoderPlugin> plugin;
+    if (lexical_mapped_file_path.extension() == "tga"sv) {
         plugin = make<TGAImageDecoderPlugin>(data, size);
         plugin = make<TGAImageDecoderPlugin>(data, size);
         if (plugin->sniff())
         if (plugin->sniff())
             return plugin;
             return plugin;
+    }
+    return {};
+}
 
 
-        return {};
-    }(data, size);
-
+RefPtr<ImageDecoder> ImageDecoder::try_create_for_raw_bytes_with_known_path(StringView path, ReadonlyBytes bytes)
+{
+    OwnPtr<ImageDecoderPlugin> plugin = probe_and_sniff_for_appropriate_plugin_with_known_path(path, bytes);
     if (!plugin)
     if (!plugin)
-        return {};
+        return try_create_for_raw_bytes(bytes);
     return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
     return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
 }
 }
 
 

+ 2 - 1
Userland/Libraries/LibGfx/ImageDecoder.h

@@ -47,7 +47,8 @@ protected:
 
 
 class ImageDecoder : public RefCounted<ImageDecoder> {
 class ImageDecoder : public RefCounted<ImageDecoder> {
 public:
 public:
-    static RefPtr<ImageDecoder> try_create(ReadonlyBytes);
+    static RefPtr<ImageDecoder> try_create_for_raw_bytes(ReadonlyBytes);
+    static RefPtr<ImageDecoder> try_create_for_raw_bytes_with_known_path(StringView path, ReadonlyBytes);
     ~ImageDecoder() = default;
     ~ImageDecoder() = default;
 
 
     IntSize size() const { return m_plugin->size(); }
     IntSize size() const { return m_plugin->size(); }

+ 37 - 0
Userland/Libraries/LibImageDecoderClient/Client.cpp

@@ -20,6 +20,43 @@ void Client::die()
         on_death();
         on_death();
 }
 }
 
 
+Optional<DecodedImage> 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<void>(), 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<DecodedImage> Client::decode_image(ReadonlyBytes encoded_data)
 Optional<DecodedImage> Client::decode_image(ReadonlyBytes encoded_data)
 {
 {
     if (encoded_data.is_empty())
     if (encoded_data.is_empty())

+ 1 - 0
Userland/Libraries/LibImageDecoderClient/Client.h

@@ -31,6 +31,7 @@ class Client final
 
 
 public:
 public:
     Optional<DecodedImage> decode_image(ReadonlyBytes);
     Optional<DecodedImage> decode_image(ReadonlyBytes);
+    Optional<DecodedImage> decode_image_with_known_path(DeprecatedString const& path, ReadonlyBytes);
 
 
     Function<void()> on_death;
     Function<void()> on_death;
 
 

+ 52 - 18
Userland/Services/ImageDecoder/ConnectionFromClient.cpp

@@ -22,40 +22,74 @@ void ConnectionFromClient::die()
     Core::EventLoop::current().quit(0);
     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<Gfx::ShareableBitmap>& bitmaps, Vector<u32>& durations)
 {
 {
-    if (!encoded_buffer.is_valid()) {
-        dbgln_if(IMAGE_DECODER_DEBUG, "Encoded data is invalid");
-        return nullptr;
+    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);
+        } else {
+            auto frame = frame_or_error.release_value();
+            bitmaps.append(frame.image->to_shareable_bitmap());
+            durations.append(frame.duration);
+        }
     }
     }
+}
+
+static void decode_image_to_details(Core::AnonymousBuffer const& encoded_buffer, Optional<StringView> known_path, bool& is_animated, u32& loop_count, Vector<Gfx::ShareableBitmap>& bitmaps, Vector<u32>& durations)
+{
+    VERIFY(bitmaps.size() == 0);
+    VERIFY(durations.size() == 0);
+    VERIFY(!is_animated);
+    VERIFY(loop_count == 0);
 
 
-    auto decoder = Gfx::ImageDecoder::try_create(ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() });
+    RefPtr<Gfx::ImageDecoder> decoder;
+    if (known_path.has_value())
+        decoder = Gfx::ImageDecoder::try_create_for_raw_bytes_with_known_path(known_path.value(), ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() });
+    else
+        decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() });
 
 
     if (!decoder) {
     if (!decoder) {
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data");
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data");
-        return { false, 0, Vector<Gfx::ShareableBitmap> {}, Vector<u32> {} };
+        return;
     }
     }
 
 
     if (!decoder->frame_count()) {
     if (!decoder->frame_count()) {
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data");
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data");
-        return { false, 0, Vector<Gfx::ShareableBitmap> {}, Vector<u32> {} };
+        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<Gfx::ShareableBitmap> bitmaps;
     Vector<Gfx::ShareableBitmap> bitmaps;
     Vector<u32> durations;
     Vector<u32> durations;
-    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);
-        } else {
-            auto frame = frame_or_error.release_value();
-            bitmaps.append(frame.image->to_shareable_bitmap());
-            durations.append(frame.duration);
-        }
+    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;
     }
     }
 
 
-    return { decoder->is_animated(), static_cast<u32>(decoder->loop_count()), bitmaps, durations };
+    bool is_animated = false;
+    u32 loop_count = 0;
+    Vector<Gfx::ShareableBitmap> bitmaps;
+    Vector<u32> durations;
+    decode_image_to_details(encoded_buffer, {}, is_animated, loop_count, bitmaps, durations);
+    return { is_animated, loop_count, bitmaps, durations };
 }
 }
 
 
 }
 }

+ 1 - 0
Userland/Services/ImageDecoder/ConnectionFromClient.h

@@ -27,6 +27,7 @@ private:
     explicit ConnectionFromClient(NonnullOwnPtr<Core::Stream::LocalSocket>);
     explicit ConnectionFromClient(NonnullOwnPtr<Core::Stream::LocalSocket>);
 
 
     virtual Messages::ImageDecoderServer::DecodeImageResponse decode_image(Core::AnonymousBuffer const&) override;
     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;
 };
 };
 
 
 }
 }

+ 1 - 0
Userland/Services/ImageDecoder/ImageDecoderServer.ipc

@@ -4,4 +4,5 @@
 endpoint ImageDecoderServer
 endpoint ImageDecoderServer
 {
 {
     decode_image(Core::AnonymousBuffer data) => (bool is_animated, u32 loop_count, Vector<Gfx::ShareableBitmap> bitmaps, Vector<u32> durations)
     decode_image(Core::AnonymousBuffer data) => (bool is_animated, u32 loop_count, Vector<Gfx::ShareableBitmap> bitmaps, Vector<u32> durations)
+    decode_image_with_known_path(DeprecatedString path, Core::AnonymousBuffer data) => (bool is_animated, u32 loop_count, Vector<Gfx::ShareableBitmap> bitmaps, Vector<u32> durations)
 }
 }

+ 1 - 1
Userland/Utilities/file.cpp

@@ -33,7 +33,7 @@ static Optional<DeprecatedString> image_details(DeprecatedString const& descript
         return {};
         return {};
 
 
     auto& mapped_file = *file_or_error.value();
     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)
     if (!image_decoder)
         return {};
         return {};
 
 

+ 1 - 1
Userland/Utilities/headless-browser.cpp

@@ -265,7 +265,7 @@ public:
 
 
     virtual Optional<Web::Platform::DecodedImage> decode_image(ReadonlyBytes data) override
     virtual Optional<Web::Platform::DecodedImage> decode_image(ReadonlyBytes data) override
     {
     {
-        auto decoder = Gfx::ImageDecoder::try_create(data);
+        auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(data);
 
 
         if (!decoder)
         if (!decoder)
             return Web::Platform::DecodedImage { false, 0, Vector<Web::Platform::Frame> {} };
             return Web::Platform::DecodedImage { false, 0, Vector<Web::Platform::Frame> {} };