Skip to content

Commit 8c8c0d8

Browse files
vitormattosvitormattos
committed
feat: harden svg parsing and test providers
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
1 parent 0940691 commit 8c8c0d8

5 files changed

Lines changed: 589 additions & 71 deletions

File tree

‎src/Pdf/Svg/SvgPdfXObjectFactory.php‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ private function parseSvgRoot(string $svgContents, string $source): DOMElement
9898
libxml_use_internal_errors($previousErrors);
9999

100100
$root = $document->documentElement;
101-
$rootName = '';
102-
if ($root instanceof DOMElement) {
103-
$rootName = strtolower($root->localName);
104-
}
101+
$rootName = $root instanceof DOMElement ? $this->normalizeLocalName($root->localName) : '';
105102

106103
if ($rootName !== 'svg') {
107104
throw new InvalidArgumentException(sprintf('Unable to parse SVG source "%s".', $source));
@@ -118,29 +115,7 @@ private function resolveViewBox(DOMElement $svg, string $source): array
118115
{
119116
$viewBox = trim($svg->getAttribute('viewBox'));
120117
if ($viewBox !== '') {
121-
$parts = preg_split('/[\s,]+/', $viewBox);
122-
if (!is_array($parts) || count($parts) !== 4) {
123-
throw new InvalidArgumentException(sprintf('Invalid viewBox in SVG source "%s".', $source));
124-
}
125-
126-
$parsedViewBox = [];
127-
128-
foreach ($parts as $part) {
129-
$parsedPart = filter_var($part, FILTER_VALIDATE_FLOAT);
130-
if (!is_float($parsedPart)) {
131-
throw new InvalidArgumentException(sprintf('Invalid viewBox in SVG source "%s".', $source));
132-
}
133-
134-
$parsedViewBox[] = $parsedPart;
135-
}
136-
137-
[$minX, $minY, $width, $height] = $parsedViewBox;
138-
139-
if ($width <= 0.0 || $height <= 0.0) {
140-
throw new InvalidArgumentException(sprintf('SVG source "%s" must define a positive viewBox.', $source));
141-
}
142-
143-
return [$minX, $minY, $width, $height];
118+
return $this->parseViewBoxValues($viewBox, $source);
144119
}
145120

146121
$width = $this->extractNumericSvgLength($svg->getAttribute('width'));
@@ -156,6 +131,36 @@ private function resolveViewBox(DOMElement $svg, string $source): array
156131
return [0.0, 0.0, $width, $height];
157132
}
158133

134+
/**
135+
* @return array{0: float, 1: float, 2: float, 3: float}
136+
*/
137+
private function parseViewBoxValues(string $viewBox, string $source): array
138+
{
139+
$parts = preg_split('/[\s,]+/', $viewBox);
140+
if (!is_array($parts) || count($parts) !== 4) {
141+
throw new InvalidArgumentException(sprintf('Invalid viewBox in SVG source "%s".', $source));
142+
}
143+
144+
$parsedViewBox = [];
145+
146+
foreach ($parts as $part) {
147+
$parsedPart = filter_var($part, FILTER_VALIDATE_FLOAT);
148+
if (!is_float($parsedPart)) {
149+
throw new InvalidArgumentException(sprintf('Invalid viewBox in SVG source "%s".', $source));
150+
}
151+
152+
$parsedViewBox[] = $parsedPart;
153+
}
154+
155+
[$minX, $minY, $width, $height] = $parsedViewBox;
156+
157+
if ($width <= 0.0 || $height <= 0.0) {
158+
throw new InvalidArgumentException(sprintf('SVG source "%s" must define a positive viewBox.', $source));
159+
}
160+
161+
return [$minX, $minY, $width, $height];
162+
}
163+
159164
private function extractNumericSvgLength(string $value): float
160165
{
161166
$trimmed = trim($value);
@@ -185,6 +190,10 @@ private function extractClassColorMaps(DOMElement $svg): array
185190
$strokes = [];
186191

187192
foreach ($svg->getElementsByTagName('style') as $styleNode) {
193+
if (!$styleNode instanceof DOMElement) {
194+
continue;
195+
}
196+
188197
$css = $styleNode->textContent;
189198
if ($css === '') {
190199
continue;
@@ -195,6 +204,7 @@ private function extractClassColorMaps(DOMElement $svg): array
195204
continue;
196205
}
197206

207+
/** @var list<array{0: string, 1: string, 2: string}> $rules */
198208
foreach ($rules as $rule) {
199209
if (preg_match('/(?:^|;)\s*fill\s*:\s*([^;]+)/i', $rule[2], $fillMatch) === 1) {
200210
$color = $this->colorResolver->normalizeColor($fillMatch[1]);
@@ -223,7 +233,11 @@ private function iterateDrawableElements(DOMElement $svg): array
223233
$elements = [];
224234

225235
foreach ($svg->getElementsByTagName('*') as $element) {
226-
$name = strtolower($element->localName);
236+
if (!$element instanceof DOMElement) {
237+
continue;
238+
}
239+
240+
$name = $this->normalizeLocalName($element->localName);
227241
if (in_array($name, ['path', 'polygon', 'polyline', 'rect', 'circle', 'ellipse', 'line'], true)) {
228242
$elements[] = $element;
229243
}
@@ -232,6 +246,11 @@ private function iterateDrawableElements(DOMElement $svg): array
232246
return $elements;
233247
}
234248

249+
private function normalizeLocalName(?string $localName): string
250+
{
251+
return strtolower($localName ?? '');
252+
}
253+
235254
private function resolveStrokeWidth(DOMElement $element): float
236255
{
237256
$attr = $element->getAttribute('stroke-width');

‎tests/Unit/Pdf/FilesystemPdfImageEmbedderTest.php‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -498,84 +498,84 @@ public function testEmbedSeparatesMultiRowRgbaPixelsIntoColorAndAlphaStreams():
498498
}
499499

500500
#[DataProvider('svgExtensionBoundaryProvider')]
501-
public function testEmbedCorrectlyDetectsSvgByFileExtensionBoundary(string $source, bool $shouldBeSvg): void
501+
public function testEmbedCorrectlyDetectsSvgByFileExtensionBoundary(string $source, array $expectedDictionary): void
502502
{
503503
$svgImage = new EmbeddedPdfImage(['Type' => '/XObject', 'Subtype' => '/Form'], 'svg-stream');
504504
$pngImage = new EmbeddedPdfImage(['Type' => '/Image'], 'png-stream');
505505
$embedder = $this->buildSvgDetectionEmbedder($svgImage, $pngImage, "\x89PNG\r\n\x1a\n");
506506

507507
$image = $embedder->embed($source);
508508

509-
if ($shouldBeSvg) {
510-
self::assertSame('/XObject', $image->dictionary['Type']);
511-
self::assertSame('/Form', $image->dictionary['Subtype']);
512-
} else {
513-
self::assertSame('/Image', $image->dictionary['Type']);
509+
foreach ($expectedDictionary as $key => $expectedValue) {
510+
self::assertSame($expectedValue, $image->dictionary[$key]);
514511
}
515512
}
516513

517514
/**
518-
* @return iterable<string, array{source: string, shouldBeSvg: bool}>
515+
* @return iterable<string, array{source: string, expectedDictionary: array<string, string>}>
519516
*/
520517
public static function svgExtensionBoundaryProvider(): iterable
521518
{
522-
yield 'SVG extension detected' => ['source' => '/path/to/file.svg', 'shouldBeSvg' => true];
523-
yield 'SVGZ extension detected' => ['source' => '/path/to/file.svgz', 'shouldBeSvg' => true];
519+
yield 'SVG extension detected' => [
520+
'source' => '/path/to/file.svg',
521+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
522+
];
523+
yield 'SVGZ extension detected' => [
524+
'source' => '/path/to/file.svgz',
525+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
526+
];
524527
yield 'SVG extension case-insensitive uppercase' => [
525528
'source' => '/path/to/file.SVG',
526-
'shouldBeSvg' => true,
529+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
527530
];
528531
yield 'SVG extension case-insensitive mixed' => [
529532
'source' => '/path/to/file.Svg',
530-
'shouldBeSvg' => true,
533+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
531534
];
532535
yield 'SVG in middle of filename not detected as SVG' => [
533536
'source' => '/path/svg.backup',
534-
'shouldBeSvg' => false,
537+
'expectedDictionary' => ['Type' => '/Image'],
535538
];
536539
yield 'SVG in filename but different extension' => [
537540
'source' => '/path/my.svg.txt',
538-
'shouldBeSvg' => false,
541+
'expectedDictionary' => ['Type' => '/Image'],
539542
];
540543
}
541544

542545
#[DataProvider('svgContentDetectionProvider')]
543-
public function testEmbedCorrectlyDetectsSvgByContentBoundary(string $content, bool $shouldBeSvg): void
546+
public function testEmbedCorrectlyDetectsSvgByContentBoundary(string $content, array $expectedDictionary): void
544547
{
545548
$svgImage = new EmbeddedPdfImage(['Type' => '/XObject', 'Subtype' => '/Form'], 'svg-stream');
546549
$pngImage = new EmbeddedPdfImage(['Type' => '/Image'], 'png-stream');
547550
$embedder = $this->buildSvgDetectionEmbedder($svgImage, $pngImage, $content);
548551

549552
$image = $embedder->embed('/path/to/image.bin');
550553

551-
if ($shouldBeSvg) {
552-
self::assertSame('/XObject', $image->dictionary['Type']);
553-
self::assertSame('/Form', $image->dictionary['Subtype']);
554-
} else {
555-
self::assertSame('/Image', $image->dictionary['Type']);
554+
foreach ($expectedDictionary as $key => $expectedValue) {
555+
self::assertSame($expectedValue, $image->dictionary[$key]);
556556
}
557557
}
558558

559559
/**
560-
* @return iterable<string, array{content: string, shouldBeSvg: bool}>
560+
* @return iterable<string, array{content: string, expectedDictionary: array<string, string>}>
561561
*/
562562
public static function svgContentDetectionProvider(): iterable
563563
{
564564
yield 'direct svg tag detected by content' => [
565565
'content' => '<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"></svg>',
566-
'shouldBeSvg' => true,
566+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
567567
];
568568
yield 'svg tag with leading whitespace is trimmed and detected' => [
569569
'content' => " \n<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"1\" height=\"1\"></svg>",
570-
'shouldBeSvg' => true,
570+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
571571
];
572572
yield 'xml declaration followed by svg tag is detected' => [
573573
'content' => '<?xml version="1.0" encoding="UTF-8"?><svg xmlns="http://www.w3.org/2000/svg"/>',
574-
'shouldBeSvg' => true,
574+
'expectedDictionary' => ['Type' => '/XObject', 'Subtype' => '/Form'],
575575
];
576576
yield 'xml declaration without svg tag is not detected as svg' => [
577577
'content' => '<?xml version="1.0"?><document><item/></document>',
578-
'shouldBeSvg' => false,
578+
'expectedDictionary' => ['Type' => '/Image'],
579579
];
580580
}
581581

‎tests/Unit/Pdf/Svg/SvgColorResolverTest.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,108 @@ public static function provideExtractColorFromStyleScenarios(): iterable
344344
yield 'whitespace style returns null' => ['style' => ' ', 'expected' => null];
345345
}
346346

347+
public function testExtractValueFromStyleAttributeSkipsOnlyWhitespaceDeclarations(): void
348+
{
349+
$resolver = new SvgColorResolver();
350+
351+
// Tab and spaces should be skipped, not just empty
352+
$result = $resolver->extractValueFromStyleAttribute(" \t \n; fill: red", 'fill');
353+
self::assertSame('red', $result);
354+
355+
// Verify leading whitespace is trimmed from properties
356+
$result = $resolver->extractValueFromStyleAttribute(" fill : blue", 'fill');
357+
self::assertSame('blue', $result);
358+
}
359+
360+
public function testExtractColorFromStyleAttributeWithAllWhitespaceDeclaration(): void
361+
{
362+
$resolver = new SvgColorResolver();
363+
364+
// All whitespace-only declarations should be skipped
365+
$result = $resolver->extractColorFromStyleAttribute(" ; \t ; \n ; fill: #123456", 'fill');
366+
self::assertSame('#123456', $result);
367+
}
368+
369+
public function testExtractValueFromStyleSkipsWhitespaceOnlyDeclarationBeforeProperty(): void
370+
{
371+
$resolver = new SvgColorResolver();
372+
373+
$style = " \t\n;stroke:none;fill:#aabbcc";
374+
$result = $resolver->extractValueFromStyleAttribute($style, 'fill');
375+
self::assertSame('#aabbcc', $result);
376+
}
377+
378+
public function testNormalizeColorAlwaysTrimsBothEndsBeforeValidation(): void
379+
{
380+
$resolver = new SvgColorResolver();
381+
382+
// Ensure leading/trailing space is always removed
383+
$result = $resolver->normalizeColor("\t #fff \n");
384+
self::assertSame('#fff', $result);
385+
386+
$result = $resolver->normalizeColor(" black \t");
387+
self::assertSame('#000000', $result);
388+
389+
$result = $resolver->normalizeColor(" rgb(10,20,30) ");
390+
self::assertSame('#0a141e', $result);
391+
}
392+
393+
public function testNormalizeColorRejectsInvalidLogicalConditions(): void
394+
{
395+
$resolver = new SvgColorResolver();
396+
397+
// Ensure both conditions in logical OR are required (not just one)
398+
$result = $resolver->normalizeColor(" ");
399+
self::assertNull($result);
400+
401+
// Non-hex starting with # should fail
402+
$result = $resolver->normalizeColor("#gggggg");
403+
self::assertNull($result);
404+
405+
// Malformed RGB should fail
406+
$result = $resolver->normalizeColor("rgb(300,300,300)");
407+
self::assertSame('#ffffff', $result); // Clamped, not rejected
408+
}
409+
410+
public function testParseRgbColorRejectionEdgeCases(): void
411+
{
412+
$resolver = new SvgColorResolver();
413+
414+
// These should all fail parsing and return null
415+
$result = $resolver->normalizeColor("rgb(invalid,0,0)");
416+
self::assertNull($result);
417+
418+
$result = $resolver->normalizeColor("rgb(-1,0,0)");
419+
self::assertNull($result);
420+
421+
$result = $resolver->normalizeColor("rgb(0,0)"); // Missing channel
422+
self::assertNull($result);
423+
424+
$result = $resolver->normalizeColor("rgb(0,0,0,0)"); // Extra channel
425+
self::assertNull($result);
426+
}
427+
428+
public function testExtractClassesNormalizationAndFiltering(): void
429+
{
430+
$resolver = new SvgColorResolver();
431+
$element = $this->createElement('div', ['class' => ' primary secondary tertiary ']);
432+
433+
// Access private method via reflection to verify extraction
434+
$method = new ReflectionMethod(SvgColorResolver::class, 'extractClasses');
435+
$method->setAccessible(true);
436+
437+
$classes = $method->invoke($resolver, ' primary secondary tertiary ');
438+
self::assertSame(['primary', 'secondary', 'tertiary'], $classes);
439+
440+
// Empty class attribute should return empty array
441+
$classes = $method->invoke($resolver, '');
442+
self::assertSame([], $classes);
443+
444+
// Whitespace-only class attribute should return empty array (preg_split with PREG_SPLIT_NO_EMPTY filters these)
445+
$classes = $method->invoke($resolver, ' ');
446+
self::assertSame([], $classes);
447+
}
448+
347449
private function createElement(string $name, array $attributes = []): DOMElement
348450
{
349451
$document = new DOMDocument('1.0', 'UTF-8');

0 commit comments

Comments
 (0)