Quellcode durchsuchen

LibGfx+Fallout: Make ImageDecoder return ErrorOr

...from try_create_for_raw_bytes().

If a plugin returns `true` from sniff but then fails when calling
its `create()` method, we now no longer swallow that error.

Allows `image` (and other places in the system) to print a more
actionable error if early image headers are invalid.

(We now no longer try to find another plugin that can also handle
the image.)

Fixes a regression from #20063 / #19893 -- before then, we didn't
do fallible work this early.
Nico Weber vor 1 Jahr
Ursprung
Commit
2e2cae26c6

+ 1 - 1
Userland/Applications/FileManager/PropertiesWindow.cpp

@@ -403,7 +403,7 @@ ErrorOr<void> PropertiesWindow::create_font_tab(GUI::TabWidget& tab_widget, Nonn
 
 ErrorOr<void> PropertiesWindow::create_image_tab(GUI::TabWidget& tab_widget, NonnullOwnPtr<Core::MappedFile> mapped_file, StringView mime_type)
 {
-    auto image_decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file->bytes(), mime_type);
+    auto image_decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file->bytes(), mime_type));
     if (!image_decoder)
         return {};
 

+ 1 - 1
Userland/Applications/ImageViewer/ViewWidget.cpp

@@ -224,7 +224,7 @@ ErrorOr<void> ViewWidget::try_open_file(String const& path, Core::File& file)
     Vector<Animation::Frame> frames;
     // Note: Doing this check only requires reading the header of images
     // (so if the image is not vector graphics it can be still be decoded OOP).
-    if (auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(file_data); decoder && decoder->natural_frame_format() == Gfx::NaturalFrameFormat::Vector) {
+    if (auto decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(file_data)); decoder && decoder->natural_frame_format() == Gfx::NaturalFrameFormat::Vector) {
         // Use in-process decoding for vector graphics.
         is_animated = decoder->is_animated();
         loop_count = decoder->loop_count();

+ 3 - 2
Userland/Applications/Maps/MapWidget.cpp

@@ -341,11 +341,12 @@ void MapWidget::process_tile_queue()
         m_first_image_loaded = true;
 
         // Decode loaded PNG image data
-        auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(payload, "image/png");
-        if (!decoder || (decoder->frame_count() == 0)) {
+        auto decoder_or_err = Gfx::ImageDecoder::try_create_for_raw_bytes(payload, "image/png");
+        if (decoder_or_err.is_error() || !decoder_or_err.value() || (decoder_or_err.value()->frame_count() == 0)) {
             dbgln("Maps: Can't decode image: {}", url);
             return;
         }
+        auto decoder = decoder_or_err.release_value();
         m_tiles.set(tile_key, decoder->frame(0).release_value_but_fixme_should_propagate_errors().image);
 
         // FIXME: only update the part of the screen that this tile covers

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

@@ -79,7 +79,7 @@ void ImageWidget::load_from_file(StringView path)
 
     auto& mapped_file = *file_or_error.value();
     auto mime_type = Core::guess_mime_type_based_on_filename(path);
-    m_image_decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file.bytes(), mime_type);
+    m_image_decoder = MUST(Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file.bytes(), mime_type));
     VERIFY(m_image_decoder);
 
     auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();

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

@@ -147,7 +147,7 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::load_from_file(NonnullOwnPtr<Core::File>
 
 ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::load_from_bytes(ReadonlyBytes bytes, Optional<IntSize> ideal_size, Optional<ByteString> mine_type)
 {
-    if (auto decoder = ImageDecoder::try_create_for_raw_bytes(bytes, mine_type)) {
+    if (auto decoder = TRY(ImageDecoder::try_create_for_raw_bytes(bytes, mine_type))) {
         auto frame = TRY(decoder->frame(0, ideal_size));
         if (auto& bitmap = frame.image)
             return bitmap.release_nonnull();

+ 6 - 8
Userland/Libraries/LibGfx/ImageFormats/ImageDecoder.cpp

@@ -26,7 +26,7 @@
 
 namespace Gfx {
 
-static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin(ReadonlyBytes bytes)
+static ErrorOr<OwnPtr<ImageDecoderPlugin>> probe_and_sniff_for_appropriate_plugin(ReadonlyBytes bytes)
 {
     struct ImagePluginInitializer {
         bool (*sniff)(ReadonlyBytes) = nullptr;
@@ -56,11 +56,9 @@ static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin(Readonl
         auto sniff_result = plugin.sniff(bytes);
         if (!sniff_result)
             continue;
-        auto plugin_decoder = plugin.create(bytes);
-        if (!plugin_decoder.is_error())
-            return plugin_decoder.release_value();
+        return TRY(plugin.create(bytes));
     }
-    return {};
+    return OwnPtr<ImageDecoderPlugin> {};
 }
 
 static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin_with_known_mime_type(StringView mime_type, ReadonlyBytes bytes)
@@ -88,9 +86,9 @@ static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin_with_kn
     return {};
 }
 
-RefPtr<ImageDecoder> ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes bytes, Optional<ByteString> mime_type)
+ErrorOr<RefPtr<ImageDecoder>> ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes bytes, Optional<ByteString> mime_type)
 {
-    if (OwnPtr<ImageDecoderPlugin> plugin = probe_and_sniff_for_appropriate_plugin(bytes); plugin)
+    if (auto plugin = TRY(probe_and_sniff_for_appropriate_plugin(bytes)); plugin)
         return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
 
     if (mime_type.has_value()) {
@@ -98,7 +96,7 @@ RefPtr<ImageDecoder> ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes bytes,
             return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
     }
 
-    return {};
+    return RefPtr<ImageDecoder> {};
 }
 
 ImageDecoder::ImageDecoder(NonnullOwnPtr<ImageDecoderPlugin> plugin)

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

@@ -98,7 +98,7 @@ protected:
 
 class ImageDecoder : public RefCounted<ImageDecoder> {
 public:
-    static RefPtr<ImageDecoder> try_create_for_raw_bytes(ReadonlyBytes, Optional<ByteString> mime_type = {});
+    static ErrorOr<RefPtr<ImageDecoder>> try_create_for_raw_bytes(ReadonlyBytes, Optional<ByteString> mime_type = {});
     ~ImageDecoder() = default;
 
     IntSize size() const { return m_plugin->size(); }

+ 3 - 2
Userland/Services/ImageDecoder/ConnectionFromClient.cpp

@@ -43,11 +43,12 @@ static void decode_image_to_details(Core::AnonymousBuffer const& encoded_buffer,
     VERIFY(bitmaps.size() == 0);
     VERIFY(durations.size() == 0);
 
-    auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() }, known_mime_type);
-    if (!decoder) {
+    auto decoder_or_err = Gfx::ImageDecoder::try_create_for_raw_bytes(ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() }, known_mime_type);
+    if (decoder_or_err.is_error() || !decoder_or_err.value()) {
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data");
         return;
     }
+    auto decoder = decoder_or_err.release_value();
 
     if (!decoder->frame_count()) {
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data");

+ 1 - 1
Userland/Services/SpiceAgent/SpiceAgent.cpp

@@ -256,7 +256,7 @@ ErrorOr<void> SpiceAgent::did_receive_clipboard_message(ClipboardMessage& messag
         auto mime_type = TRY(clipboard_data_type_to_mime_type(message.data_type()));
 
         // FIXME: It should be trivial to make `try_create_for_raw_bytes` take a `StringView` instead of a direct `ByteString`.
-        auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(message.contents(), mime_type.to_byte_string());
+        auto decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(message.contents(), mime_type.to_byte_string()));
         if (!decoder || (decoder->frame_count() == 0)) {
             return Error::from_string_literal("Failed to find a suitable decoder for a pasted image!");
         }

+ 1 - 1
Userland/Utilities/file.cpp

@@ -33,7 +33,7 @@ static ErrorOr<Optional<String>> image_details(StringView description, StringVie
 {
     auto mapped_file = TRY(Core::MappedFile::map(path));
     auto mime_type = Core::guess_mime_type_based_on_filename(path);
-    auto image_decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file->bytes(), mime_type);
+    auto image_decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(mapped_file->bytes(), mime_type));
     if (!image_decoder)
         return OptionalNone {};
 

+ 1 - 1
Userland/Utilities/icc.cpp

@@ -276,7 +276,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         }
         auto file = TRY(Core::MappedFile::map(path));
 
-        auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(file->bytes());
+        auto decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(file->bytes()));
         if (decoder) {
             if (auto embedded_icc_bytes = TRY(decoder->icc_data()); embedded_icc_bytes.has_value()) {
                 icc_bytes = *embedded_icc_bytes;

+ 2 - 2
Userland/Utilities/image.cpp

@@ -227,9 +227,9 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     Options options = TRY(parse_options(arguments));
 
     auto file = TRY(Core::MappedFile::map(options.in_path));
-    auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(file->bytes());
+    auto decoder = TRY(Gfx::ImageDecoder::try_create_for_raw_bytes(file->bytes()));
     if (!decoder)
-        return Error::from_string_view("Failed to decode input file"sv);
+        return Error::from_string_view("Could not find decoder for input file"sv);
 
     LoadedImage image = TRY(load_image(*decoder, options.frame_index));