From 68a307be4e08df6218483ecf1cd92677241c8d18 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 12 Jun 2021 11:17:52 +0200 Subject: [PATCH] PixelPaint: Use ImageDecoder to load images out-of-process This sandboxes the image decoding work done by PixelPaint to prevent bugs in the decoding framework from compromising PixelPaint itself. :^) --- .../Applications/PixelPaint/CMakeLists.txt | 2 +- Userland/Applications/PixelPaint/Image.cpp | 42 +++++++++++++++---- Userland/Applications/PixelPaint/main.cpp | 5 --- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/Userland/Applications/PixelPaint/CMakeLists.txt b/Userland/Applications/PixelPaint/CMakeLists.txt index fae25a2267..6b1d34103f 100644 --- a/Userland/Applications/PixelPaint/CMakeLists.txt +++ b/Userland/Applications/PixelPaint/CMakeLists.txt @@ -28,4 +28,4 @@ set(SOURCES ) serenity_app(PixelPaint ICON app-pixel-paint) -target_link_libraries(PixelPaint LibGUI LibGfx) +target_link_libraries(PixelPaint LibImageDecoderClient LibGUI LibGfx) diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp index 7b24f06967..366171a411 100644 --- a/Userland/Applications/PixelPaint/Image.cpp +++ b/Userland/Applications/PixelPaint/Image.cpp @@ -10,18 +10,34 @@ #include #include #include +#include #include #include -#include #include #include -#include -#include #include +#include #include namespace PixelPaint { +static RefPtr try_decode_bitmap(ByteBuffer const& bitmap_data) +{ + // Spawn a new ImageDecoder service process and connect to it. + auto client = ImageDecoderClient::Client::construct(); + + // FIXME: Find a way to avoid the memory copying here. + auto decoded_image_or_error = client->decode_image(bitmap_data); + if (!decoded_image_or_error.has_value()) + return nullptr; + + // FIXME: Support multi-frame images? + auto decoded_image = decoded_image_or_error.release_value(); + if (decoded_image.frames.is_empty()) + return nullptr; + return move(decoded_image.frames[0].bitmap); +} + RefPtr Image::try_create_with_size(Gfx::IntSize const& size) { if (size.is_empty()) @@ -97,8 +113,8 @@ RefPtr Image::try_create_from_pixel_paint_file(String const& file_path) auto bitmap_base64_encoded = json_layer_object.get("bitmap").as_string(); auto bitmap_data = decode_base64(bitmap_base64_encoded); - auto image_decoder = Gfx::ImageDecoder::create(bitmap_data); - auto bitmap = image_decoder->bitmap(); + + auto bitmap = try_decode_bitmap(bitmap_data); VERIFY(bitmap); layer->set_bitmap(bitmap.release_nonnull()); image->add_layer(*layer); @@ -109,9 +125,19 @@ RefPtr Image::try_create_from_pixel_paint_file(String const& file_path) RefPtr Image::try_create_from_file(String const& file_path) { - if (auto bitmap = Gfx::Bitmap::load_from_file(file_path)) - return try_create_from_bitmap(bitmap.release_nonnull()); - return try_create_from_pixel_paint_file(file_path); + if (auto image = try_create_from_pixel_paint_file(file_path)) + return image; + + auto file_or_error = MappedFile::map(file_path); + if (file_or_error.is_error()) + return nullptr; + + auto& mapped_file = *file_or_error.value(); + // FIXME: Find a way to avoid the memory copy here. + auto bitmap = try_decode_bitmap(ByteBuffer::copy(mapped_file.bytes())); + if (!bitmap) + return nullptr; + return Image::try_create_from_bitmap(bitmap.release_nonnull()); } void Image::save(String const& file_path) const diff --git a/Userland/Applications/PixelPaint/main.cpp b/Userland/Applications/PixelPaint/main.cpp index 6617a2011d..1006697e8e 100644 --- a/Userland/Applications/PixelPaint/main.cpp +++ b/Userland/Applications/PixelPaint/main.cpp @@ -42,11 +42,6 @@ int main(int argc, char** argv) auto app = GUI::Application::construct(argc, argv); - if (pledge("stdio thread recvfd sendfd rpath wpath cpath", nullptr) < 0) { - perror("pledge"); - return 1; - } - const char* image_file = nullptr; Core::ArgsParser args_parser; args_parser.add_positional_argument(image_file, "Image file to open", "path", Core::ArgsParser::Required::No);