[FVWM 1.1.1] Crash when using SVG images {fvwm3: calloc: zero size}

[sebaro]

Thanks for reporting your bug here! The following template will help with
giving as much information as possible so that it's easier to diagnose and
fix.

Upfront Information

Please provide the following information by running the command and providing
the output.

• Fvwm3 version (run: fvwm3 --version)
  fvwm3 1.1.1 (released)
  with support for: ReadLine, XPM, PNG, SVG, Shape, XShm, SM, XRandR, XRender, XCursor, XFT, XFixes, NLS

• Linux distribution or BSD name/version
  Gentoo

• Platform (run: uname -sp)
  Linux AMD Athlon(tm) 5350 APU with Radeon(tm) R3

Steps to Reproduce

+ "Minim%/home/.fvwm/artwork/icons/places/32/folder.png%" ChangeIconTheme "Minim"
+ "Breeze%/usr/share/icons/breeze/places/32/folder.svg%" ChangeIconTheme "Breeze"
+ "Breeze-dark%/usr/share/icons/breeze-dark/places/32/folder.svg%" ChangeIconTheme "Breeze-dark"`

Does Fvwm3 crash?

Yes

Extra Information

librsvg-2.57.3

[somiaj]

If you specify the image size, does it work, for example:

+ "Breeze%/usr/share/icons/breeze/places/32/folder.svg:32x32%" ChangeIconTheme "Breeze"

And if that works, do the SVG images include a default height="" and width="" tag?

[sebaro]

With image size it doesn't crash but the images are not shown (I assume not found):

+ "Minim%/home/.fvwm/artwork/icons/places/32/folder.png%" ChangeIconTheme "Minim"
+ "Breeze%/usr/share/icons/breeze/places/32/folder.svg:32x32%" ChangeIconTheme "Breeze"

The issue is with FvwmScript too:

  Set $item1Icon = $iconDir {user-home.svg}
  ChangeIcon 1 $item1Icon

user-home.svg

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32">
  <defs id="defs3051">
    <style type="text/css" id="current-color-scheme">
      .ColorScheme-Text {
        color:#31363b;
      }
      .ColorScheme-Highlight {
        color:#3daee9;
      }
      </style>
  </defs>
 <path
     style="fill:currentColor;fill-opacity:1;stroke:none"
     d="M 2 3 L 2 10 L 1 10 L 1 29 L 12 29 L 13 29 L 31 29 L 31 8 L 30 8 L 30 5 L 16 5 L 14 3 L 2 3 z "
     class="ColorScheme-Highlight"
     />
 <path
     style="fill-opacity:0.33;fill-rule:evenodd"
     d="m 2,3 0,7 9,0 L 13,8 30,8 30,5 16,5 14,3 2,3 Z"
     />
 <path
     style="fill:#ffffff;fill-opacity:0.2;fill-rule:evenodd"
     d="M 14 3 L 15 6 L 30 6 L 30 5 L 16 5 L 14 3 z M 13 8 L 11 10 L 1 10 L 1 11 L 12 11 L 13 8 z "
     />
 <path
     style="fill-opacity:0.2;fill-rule:evenodd"
     d="M 13 8 L 11 9 L 2 9 L 2 10 L 11 10 L 13 8 z M 1 28 L 1 29 L 31 29 L 31 28 L 1 28 z "
     class="ColorScheme-Text"
     />
 <path
     style="fill:currentColor;fill-opacity:0.6;stroke:none"
     d="M 16 13 L 11 18 L 11 19 L 12 19 L 12 22 L 12 23 L 20 23 L 20 22 L 20 19 L 21 19 L 21 18 L 19 16 L 19 14 L 18 14 L 18 15 L 16 13 z M 16 14.4 L 19.6 18 L 19 18 L 19 19 L 19 22 L 17 22 L 17 19 L 15 19 L 15 22 L 13 22 L 13 19 L 13 18 L 12.4 18 L 16 14.4 z "
     class="ColorScheme-Text"
     />
</svg>

[somiaj]

If you revert this commit, do things start working as expected?

43870cd

[sebaro]

If you revert this commit, do things start working as expected?

43870cd

Yes

[somiaj]

@ThomasAdam Maybe you can look closer at that commit, I'm unsure what could be going on with the API change that was added.

[sebaro]

This happens only with SVG images that do not have the dimensions defined:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32">

No crash after adding dimensions:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32" width="32" height="32">

[somiaj]

That is what I thought, it looked like some division by zero error. Looks like the old deprecated methods dealt with that case while our newer code doesn't. Thanks, this does give us something to look into.

[ThomasAdam]

Right, so according to this: https://gnome.pages.gitlab.gnome.org/librsvg/Rsvg-2.0/method.Handle.get_intrinsic_size_in_pixels.html

The API has changed:

This function is not able to extract the size in pixels directly from the intrinsic dimensions of the SVG document if the width or height are in percentage units (or if they do not exist, in which case the SVG spec mandates that they default to 100%), as these require a viewport to be resolved to a final size. In this case, the function returns FALSE.

So I've got some work to do here to choose the appropriate values.

When I tested this, I'd only ever had SVGs set with an explicit width=, height= -- I didn't realise these could be omitted.

[ThomasAdam]

I'm working on this on ta/gh-1133 -- it's not ready yet.

[ThomasAdam]

Hi @sebaro

Would you mind trying out the patch on the ta/gh-1133 branch for me?

From what I can tell, this seems to be "working" -- whether it's doing the correct thing in all cases, I'm not sure. But in my testing of eliding the width/height attributes, and relying just on the viewbox attribute, things seem to behave.

[sebaro]

Hi @sebaro

Would you mind trying out the patch on the ta/gh-1133 branch for me?

From what I can tell, this seems to be "working" -- whether it's doing the correct thing in all cases, I'm not sure. But in my testing of eliding the width/height attributes, and relying just on the viewbox attribute, things seem to behave.

Yes, it's fine with this branch. Thanks.