Report an SVG the XML parser cannot read instead of dividing by zero

[jakejackson1]

Summary

A follow-up to #41, though it stands on its own and targets gravitypdf. Not a mirror of anything upstream.

An attribute value with no quotes round it is well formed in HTML and not in XML. AdjustHTML() lifts an inline <svg>…</svg> out of the HTML, writes it to a temporary file and points an <img> at it, and Image\Svg::ImageSVG() then hands that file to an expat parser — which gives up on the <svg> element itself:

<svg width="40mm" height="20mm" class=narrow xmlns="http://www.w3.org/2000/svg">…</svg>

svgOffset() is what sets svg_info['x'], ['y'], ['w'] and ['h'], and it is the <svg> element's own start handler, so nothing set them. xml_parse()'s return value is discarded, and the method read all four anyway:

xml_parse($svg2pdf_xml_parser, $data);

if ($this->svg_error) {
    return false;
} else {
    return [
        'x' => $this->svg_info['x'] * $this->kp,

Four Undefined array key warnings, a width of zero handed back to Tag\Img, and then Division by zero at Img.php:372 — which is fatal, so the whole document is lost over one picture.

svg_error was the way out all along and nothing ever set it. ImageProcessor::processSvg() already turns a false into imageError($file, $firstTime, 'Error parsing SVG file'): a broken-image placeholder and a logged warning normally, an MpdfImageException where showImageErrors or debug is on. So set the flag when the parse leaves no dimensions behind, and an unreadable SVG is treated like any other picture that cannot be read.

Try it

$mpdf = new \Mpdf\Mpdf();
$mpdf->WriteHTML(
    '<p>Before the picture</p>'
    . '<svg width="40mm" height="20mm" class=narrow xmlns="http://www.w3.org/2000/svg">'
    . '<circle cx="20" cy="20" r="15" fill="red" /></svg>'
    . '<p>After the picture</p>'
);
$mpdf->Output('try.pdf', 'F');

Before: four Warning: Undefined array key in Svg.php, then PHP Fatal error: Uncaught DivisionByZeroError: Division by zero in src/Tag/Img.php:372. No file is written.
After: a PDF with both paragraphs and a broken-image placeholder where the circle would have been. With $mpdf->showImageErrors = true; it raises Mpdf\MpdfImageException: Error parsing SVG file instead, naming the file.

Test plan

tests/Mpdf/Image/SvgErrorTest.php, eight cases. Three fail on gravitypdf, five are controls that pass on both.

• testAnSvgTheParserCannotReadDoesNotBringTheDocumentDown — the document is written and both paragraphs are in it. DivisionByZeroError on gravitypdf.
• testItRaisesNothingOfItsOwn — installs a set_error_handler and asserts it collected nothing. Four warnings on gravitypdf.
• testItIsReportedAsAnImageErrorLikeAnyOtherPictureThatCannotBeRead — with showImageErrors on, MpdfImageException carrying Error parsing SVG file. On gravitypdf the warning fires first.
• testAWellFormedSvgIsStillDrawn, testAnSvgWithNoWidthOrHeightIsStillDrawn, testAnSvgWhoseViewBoxIsAllZeroesIsStillDrawn, testAnSvgWithNothingToDrawInItIsStillNotAnError — controls. Each asserts a form XObject reaches the page, which is how an SVG is drawn.
• testAnSvgWithAnElementLeftOpenIsStillDrawn — the control that matters most; see below.

All twenty-two stored snapshot documents render pixel-identical at the harness's 120 dpi, so nothing that draws today stops drawing. No new snapshot: the only new output is mPDF's existing broken-image placeholder.

composer test — 1130 tests, 2653 assertions, up from 1122/2643. composer cs clean. phpstan output identical to gravitypdf.

More info — why not just check what xml_parse() returns

xml_parse($parser, $data) is called once, without $is_final, so expat is still waiting for more input when the call comes back and never gets round to complaining about anything left open at the end. Documents that rely on that render today. Refusing on the return value alone would be tempting, and passing true and refusing would be stricter still, but both would newly reject SVGs that currently draw — browsers are lenient about this too, and mPDF's own regex passes over the markup beforehand (stripping <pattern> and <marker> blocks, rewriting <use> into <g>) can leave things unbalanced on their own.

Testing for the dimensions instead is the narrower question and the one that matches the symptom: it is true exactly when the parser never reached the <svg> element, which is the only case that produced a zero-sized image. testAnSvgWithAnElementLeftOpenIsStillDrawn pins that a document with an element left open is not newly refused.

A viewBox of all zeroes and a width/height of zero both already fall through svgOffset() to the default sizing and were never part of this; two controls hold them there.