10up / safe-svg

Enable SVG uploads and sanitize them to stop XML/SVG vulnerabilities in your WordPress website.
https://wordpress.org/plugins/safe-svg/
GNU General Public License v2.0
271 stars 31 forks source link

Meta flood for zero-sizes #24

Open vralle opened 2 years ago

vralle commented 2 years ago

Describe the bug

The easiest way to remove thumbnail generation of a image size is to set the width and height to 0. This is an example of wp_get_additional_image_sizes() values:

array (
  '1536x1536' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  '2048x2048' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  'medium_large' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  'woocommerce_gallery_thumbnail' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  'shop_catalog' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  'shop_single' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
  'shop_thumbnail' => 
  array (
    'width' => 0,
    'height' => 0,
    'crop' => 0,
  ),
);

WordPress skips generation of such thumbnails. The plugin generates thumbnail metadata, where width and height can be 0.

This behavior results in unexpected behavior when using thumbnails. For example, structured data generation plugins can use zero-sizes for thumbnails.

This behavior is related #23

Steps to Reproduce

  1. Disable image size by simply step:
    add_image_size( 'medium_large', 0, 0, 0 );
  2. Upload the SVG image
  3. See generated metadata for attachment

Screenshots, screen recording, code snippet

No response

Environment information

No response

WordPress information

No response

Code of Conduct

jeffpaul commented 2 years ago

@darylldoyle any thoughts on best approach here for an engineer to help with?

vralle commented 2 years ago

Here's what I remember. I could not find the need to generate metadata for the SVG. It is always a link to one file. We can generate an image tag of any size. Only the width and height attributes change. Sizes is available through filters. I may be wrong, but I've come to the conclusion that there is no need for persistent image size metadata. Instead, we can use a metadata filter that can return any size on request. The downside of this approach is that we won't be able to set dimensions for removed dimensions. This scenario is possible in case of using outdated sizes in templates/rest-client. Probably, in this case, you can generate a default size with the hope that this will not lead to a violation of the layout.

vralle commented 2 years ago

Here is what I did: https://gist.github.com/vralle/f45d68d758e2d26e54b99b71fa13f5e4

  1. SVG sizes in metadata is no longer needed
  2. Image tags are not using safe_svg->svg_dimensions
  3. Image sizes are generated upon request. Image aspect ratio is preserved (responsive sizes require additional code)