diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d7d467a..d0044bbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Latest * [#198](https://github.com/cleverage/process-bundle/issues/198) Fix DateParserTransformer: a `\DateTimeImmutable` input is converted to a `\DateTime` instead of throwing a `TypeError`. Update documentation, add tests. * [#199](https://github.com/cleverage/process-bundle/issues/199) Fix PregFilterTransformer: an array `replacement` is no longer cast to the string `"Array"`, and requires an array `pattern`. Update documentation, add tests. * [#200](https://github.com/cleverage/process-bundle/issues/200) Fix GenericTransformer: a contextual option declared with `required: false` and no default can now be used; its placeholders are replaced by `null` when omitted. Update documentation, add tests. +* [#202](https://github.com/cleverage/process-bundle/issues/202) Fix XmlReaderTask: throw an explicit `\UnexpectedValueException` on an empty file or invalid XML (with the libxml error messages) instead of a `ValueError` or a silent empty `\DOMDocument`. Update documentation, add tests. ## Deprecated * [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0. diff --git a/docs/reference/tasks/xml_reader_task.md b/docs/reference/tasks/xml_reader_task.md index 37b96901..e2f43796 100644 --- a/docs/reference/tasks/xml_reader_task.md +++ b/docs/reference/tasks/xml_reader_task.md @@ -43,6 +43,7 @@ read_xml: Notes ----- -* The result of `loadXML()` is not checked: an invalid XML content raises libxml warnings (which may be converted to - exceptions by the Symfony error handler) and produces an empty or partial document. -* An empty file raises a `\ValueError` (the file content is read with `fread()` using the file size as length). +* An empty file throws an `\UnexpectedValueException`. +* An invalid XML content (not well-formed, undefined namespace prefix...) throws an `\UnexpectedValueException` whose + message contains the libxml error(s) with their line and column. libxml warnings are tolerated. The libxml internal + errors setting is restored after loading. diff --git a/src/Filesystem/XmlFile.php b/src/Filesystem/XmlFile.php index 99f20607..9bb9bca8 100644 --- a/src/Filesystem/XmlFile.php +++ b/src/Filesystem/XmlFile.php @@ -27,12 +27,39 @@ public function __construct(string $path, string $mode = 'rb') public function read(): \DOMDocument { - $dom = new \DOMDocument(); $this->file->rewind(); - $fileSize = $this->file->getSize(); + // fstat() on the open handle, as getSize() may return a stale size from the stat cache + $fileSize = $this->file->fstat()['size']; + if (0 === $fileSize) { + throw new \UnexpectedValueException(\sprintf('XML file "%s" is empty', $this->file->getPathname())); + } + $fileContent = $this->file->fread($fileSize); + if (false === $fileContent) { + throw new \RuntimeException(\sprintf('Could not read content from XML file "%s"', $this->file->getPathname())); + } - $dom->loadXML($fileContent); + $dom = new \DOMDocument(); + $previousUseErrors = libxml_use_internal_errors(true); + libxml_clear_errors(); + try { + $loaded = $dom->loadXML($fileContent); + $errors = libxml_get_errors(); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previousUseErrors); + } + + // Warnings are tolerated, errors (e.g. undefined namespace prefix) and fatal errors are not + $errors = array_filter($errors, static fn (\LibXMLError $error): bool => \LIBXML_ERR_WARNING !== $error->level); + if (!$loaded || [] !== $errors) { + $messages = array_map( + static fn (\LibXMLError $error): string => \sprintf('%s (line %d, column %d)', trim($error->message), $error->line, $error->column), + $errors, + ); + + throw new \UnexpectedValueException(\sprintf('Invalid XML in file "%s": %s', $this->file->getPathname(), [] !== $messages ? implode('; ', $messages) : 'unknown error')); + } return $dom; } diff --git a/tests/Filesystem/XmlFileTest.php b/tests/Filesystem/XmlFileTest.php new file mode 100644 index 00000000..81a1fa2c --- /dev/null +++ b/tests/Filesystem/XmlFileTest.php @@ -0,0 +1,118 @@ +path = $path; + } + + protected function tearDown(): void + { + if (is_file($this->path)) { + unlink($this->path); + } + } + + public function testReadValidXml(): void + { + file_put_contents($this->path, '1'); + + $dom = (new XmlFile($this->path))->read(); + + self::assertNotNull($dom->documentElement); + self::assertSame('root', $dom->documentElement->nodeName); + self::assertSame('1', $dom->documentElement->textContent); + } + + public function testReadEmptyFileThrows(): void + { + file_put_contents($this->path, ''); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage(\sprintf('XML file "%s" is empty', $this->path)); + + (new XmlFile($this->path))->read(); + } + + public function testReadMalformedXmlThrowsWithLibxmlMessage(): void + { + file_put_contents($this->path, '1'); + + try { + (new XmlFile($this->path))->read(); + self::fail('An exception should have been thrown'); + } catch (\UnexpectedValueException $e) { + self::assertStringContainsString(\sprintf('Invalid XML in file "%s"', $this->path), $e->getMessage()); + self::assertStringContainsString('Premature end of data in tag root', $e->getMessage()); + } + } + + public function testReadNotXmlThrows(): void + { + file_put_contents($this->path, 'not xml at all'); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage("Start tag expected, '<' not found"); + + (new XmlFile($this->path))->read(); + } + + public function testReadUndefinedNamespacePrefixThrows(): void + { + file_put_contents($this->path, ''); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Namespace prefix x on a is not defined'); + + (new XmlFile($this->path))->read(); + } + + public function testReadRestoresLibxmlErrorHandling(): void + { + file_put_contents($this->path, ''); + $previous = libxml_use_internal_errors(false); + + try { + (new XmlFile($this->path))->read(); + self::fail('An exception should have been thrown'); + } catch (\UnexpectedValueException) { + self::assertFalse(libxml_use_internal_errors()); + self::assertSame([], libxml_get_errors()); + } finally { + libxml_use_internal_errors($previous); + } + } + + public function testWriteThenRead(): void + { + $dom = new \DOMDocument(); + $dom->loadXML('ok'); + + (new XmlFile($this->path, 'wb'))->write($dom); + $read = (new XmlFile($this->path))->read(); + + self::assertSame('ok', $read->documentElement?->textContent); + } +}