fix: trigger onerror while svg image is invalid

__test__/image.spec.ts

@@ -143,3 +143,22 @@ test('reset src to empty should not throw error', async (t) => {
       }),
   )
 })
+
+test('load invalid svg should throw error', async (t) => {
+  await t.throwsAsync(
+    () =>
+      new Promise((_, reject) => {
+        const image = new Image()
+        image.onload = () => {
+          reject(new Error('should not be called'))
+        }
+        image.onerror = (err) => {
+          reject(err)
+        }
+        image.src = Buffer.from('<svg></svg><p></p>')
+      }),
+    {
+      message: /Invalid/,
+    },
+  )
+})

skia-c/skia_c.cpp

@@ -1520,24 +1520,35 @@ extern "C"
     bitmap_info->height = height;
   }
 
-  void skiac_bitmap_make_from_svg(const uint8_t *data, size_t length, float width, float height, skiac_bitmap_info *bitmap_info, skiac_font_collection *c_collection, uint8_t cs)
+  bool skiac_bitmap_make_from_svg(
+    const uint8_t *data,
+    size_t length,
+    float width,
+    float height,
+    skiac_bitmap_info *bitmap_info,
+    skiac_font_collection *c_collection,
+    uint8_t cs)
   {
     auto color_space = COLOR_SPACE_CAST;
     auto svg_stream = new SkMemoryStream(data, length, false);
     auto svg_dom = SkSVGDOM::Builder().setFontManager(c_collection->assets).make(*svg_stream);
+    if (!svg_dom)
+    {
+      return false;
+    }
     auto svg_root = svg_dom->getRoot();
     auto svg_container_size = svg_root->intrinsicSize(SkSVGLengthContext(SkSize::Make(0, 0)));
     if (svg_container_size.isZero())
     {
       auto view_box = svg_root->getViewBox();
       if (!view_box.isValid())
       {
-        return;
+        return true;
       }
       svg_container_size = SkSize::Make(view_box->width(), view_box->height());
       if (svg_container_size.isEmpty())
       {
-        return;
+        return true;
       }
       svg_dom->setContainerSize(svg_container_size);
     }
@@ -1557,6 +1568,7 @@ extern "C"
     bitmap_info->bitmap = reinterpret_cast<skiac_bitmap *>(bitmap);
     bitmap_info->width = imageinfo.width();
     bitmap_info->height = imageinfo.height();
+    return true;
   }
 
   skiac_bitmap *skiac_bitmap_make_from_image_data(uint8_t *ptr, size_t width, size_t height, size_t row_bytes, size_t size, int ct, int at)

skia-c/skia_c.hpp

@@ -474,7 +474,7 @@ extern "C"
 
   // Bitmap
   void skiac_bitmap_make_from_buffer(const uint8_t *ptr, size_t size, skiac_bitmap_info *bitmap_info);
-  void skiac_bitmap_make_from_svg(const uint8_t *data, size_t length, float width, float height, skiac_bitmap_info *bitmap_info, skiac_font_collection *c_collection, uint8_t cs);
+  bool skiac_bitmap_make_from_svg(const uint8_t *data, size_t length, float width, float height, skiac_bitmap_info *bitmap_info, skiac_font_collection *c_collection, uint8_t cs);
   skiac_bitmap *skiac_bitmap_make_from_image_data(uint8_t *ptr, size_t width, size_t height, size_t row_bytes, size_t size, int ct, int at);
   size_t skiac_bitmap_get_width(skiac_bitmap *c_bitmap);
   size_t skiac_bitmap_get_height(skiac_bitmap *c_bitmap);

src/error.rs

@@ -31,6 +31,8 @@ pub enum SkError {
   InvalidTransform(Matrix),
   #[error("Convert String to CString failed")]
   NulError,
+  #[error("Invalid SVG")]
+  InvalidSvg,
   #[error("[`{0}`] is not valid font style")]
   InvalidFontStyle(String),
   #[error("[`{0}`] is not valid font variant")]

src/image.rs

@@ -238,11 +238,11 @@ impl Image {
               image_binary.len(),
             ))
           } else {
-            self.on_error(env, &this)?;
+            self.on_error(env, &this, None)?;
             None
           }
         } else {
-          self.on_error(env, &this)?;
+          self.on_error(env, &this, None)?;
           None
         }
       } else {
@@ -256,19 +256,33 @@ impl Image {
       self.is_svg = true;
       let font = get_font().map_err(SkError::from)?;
       if (self.width - -1.0).abs() > f64::EPSILON && (self.height - -1.0).abs() > f64::EPSILON {
-        Bitmap::from_svg_data_with_custom_size(
+        if let Some(bitmap) = Bitmap::from_svg_data_with_custom_size(
           data.as_ptr(),
           length,
           self.width as f32,
           self.height as f32,
           self.color_space,
           &font,
-        )
-      } else {
+        ) {
+          Some(bitmap)
+        } else {
+          self.on_error(env, &this, Some("Invalid SVG image"))?;
+          None
+        }
+      } else if let Some(bitmap) =
         Bitmap::from_svg_data(data.as_ptr(), length, self.color_space, &font)
+      {
+        if let Ok(bitmap) = bitmap {
+          Some(bitmap)
+        } else {
+          self.on_error(env, &this, Some("Invalid SVG image"))?;
+          None
+        }
+      } else {
+        None
       }
     } else {
-      self.on_error(env, &this)?;
+      self.on_error(env, &this, None)?;
       None
     };
     if let Some(ref b) = bitmap {
@@ -312,9 +326,10 @@ impl Image {
     Ok(())
   }
 
-  fn on_error(&self, env: Env, this: &This) -> Result<()> {
+  fn on_error(&self, env: Env, this: &This, msg: Option<&str>) -> Result<()> {
     let onerror = this.get_named_property_unchecked::<Unknown>("onerror")?;
-    let err = Error::new(Status::InvalidArg, "Unsupported image type");
+    let reason = msg.unwrap_or("Unsupported image type");
+    let err = Error::new(Status::InvalidArg, reason);
     if onerror.get_type()? == ValueType::Function {
       let onerror_func = unsafe { onerror.cast::<JsFunction>() };
       onerror_func.call(Some(this), &[JsError::from(err).into_unknown(env)])?;

src/sk.rs

@@ -817,7 +817,7 @@ pub mod ffi {
       info: *mut skiac_bitmap_info,
       font_collection: *mut skiac_font_collection,
       cs: u8,
-    );
+    ) -> bool;
 
     pub fn skiac_bitmap_make_from_image_data(
       ptr: *mut u8,
@@ -3430,14 +3430,14 @@ impl Bitmap {
     size: usize,
     color_space: ColorSpace,
     fc: &FontCollection,
-  ) -> Option<Self> {
+  ) -> Option<Result<Self, crate::error::SkError>> {
     let mut bitmap_info = ffi::skiac_bitmap_info {
       bitmap: ptr::null_mut(),
       width: 0,
       height: 0,
     };
     unsafe {
-      ffi::skiac_bitmap_make_from_svg(
+      let is_valid = ffi::skiac_bitmap_make_from_svg(
         data,
         size,
         -1.0,
@@ -3447,10 +3447,14 @@ impl Bitmap {
         color_space as u8,
       );
 
+      if !is_valid {
+        return Some(Err(crate::error::SkError::InvalidSvg));
+      }
+
       if bitmap_info.bitmap.is_null() {
         return None;
       }
-      Some(Bitmap(bitmap_info))
+      Some(Ok(Bitmap(bitmap_info)))
     }
   }