ampproject/amp-toolbox-php

SelfClosingSVGElements issues with large SVGs

Closed this issue · 4 comments

swissspidy commented

In the Web Stories plugin we deal with potentially very large SVGs, like this one:

https://github.com/google/web-stories-wp/blob/be95b3c991e2fb6209be4f81650f8d75322f76b1/packages/stickers/src/indoor-garden-oasis/floral.js (over 400K)

This causes the preg_replace() call in SelfClosingSVGElements to fail and return null. That is not ideal, as it will basically replace the whole existing HTML string with null.

Proposed solution: check return value of preg_replace() first instead of simply blindly returning. This goes for other filters too.

Second, this filter's #<(circle|g|path)([^>]*?)(?>\s*(?<!\\)/)?>(?!.*</\1>)# regex seems to be very inefficient. When applying it on the above SVG on regex101.com, it errors with Catastrophic backtracking has been detected and the execution of your expression has been halted

Proposed solution: make regex more performant when dealing with such large elements

schlessera commented

Thanks for the report, @swissspidy, I'll look into this now.

  • ❤️1
swissspidy commented

@schlessera Confirmed! Will have to wait ampproject/amp-wp#6792 though to update our plugin because of the pinned version there.

  • 👍1
westonruter commented

Now merged.