diff --git a/composer.json b/composer.json index aac8c14ec44..90ab64b6143 100644 --- a/composer.json +++ b/composer.json @@ -54,6 +54,7 @@ "doctrine/annotations": "<1.13 || >= 2.0" }, "suggest": { + "ext-dom": "Provides support for XSD validation for XML mapping files", "symfony/cache": "Provides cache support for Setup Tool with doctrine/cache 2.0", "symfony/yaml": "If you want to use YAML Metadata Mapping Driver" }, diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index d6223a92040..b46db164c1d 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -5,11 +5,13 @@ namespace Doctrine\ORM\Mapping\Driver; use Doctrine\Common\Collections\Criteria; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\MappingException; use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata; use Doctrine\Persistence\Mapping\Driver\FileDriver; +use DOMDocument; use InvalidArgumentException; use LogicException; use SimpleXMLElement; @@ -22,6 +24,9 @@ use function extension_loaded; use function file_get_contents; use function in_array; +use function libxml_clear_errors; +use function libxml_get_errors; +use function libxml_use_internal_errors; use function simplexml_load_string; use function sprintf; use function str_replace; @@ -36,10 +41,13 @@ class XmlDriver extends FileDriver { public const DEFAULT_FILE_EXTENSION = '.dcm.xml'; + /** @var bool */ + private $isXsdValidationEnabled; + /** * {@inheritDoc} */ - public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENSION) + public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENSION, bool $isXsdValidationEnabled = false) { if (! extension_loaded('simplexml')) { throw new LogicException(sprintf( @@ -48,6 +56,25 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS )); } + if (! $isXsdValidationEnabled) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/6728', + sprintf( + 'Using XML mapping driver with XSD validation disabled is deprecated' + . ' and will not be supported in Doctrine ORM 3.0.' + ) + ); + } + + if ($isXsdValidationEnabled && ! extension_loaded('dom')) { + throw new LogicException(sprintf( + 'XSD validation cannot be enabled because the DOM extension is missing.' + )); + } + + $this->isXsdValidationEnabled = $isXsdValidationEnabled; + parent::__construct($locator, $fileExtension); } @@ -940,6 +967,7 @@ private function getCascadeMappings(SimpleXMLElement $cascadeElement): array */ protected function loadMappingFile($file) { + $this->validateMapping($file); $result = []; // Note: we do not use `simplexml_load_file()` because of https://bugs.php.net/bug.php?id=62577 $xmlElement = simplexml_load_string(file_get_contents($file)); @@ -967,6 +995,27 @@ protected function loadMappingFile($file) return $result; } + private function validateMapping(string $file): void + { + if (! $this->isXsdValidationEnabled) { + return; + } + + $backedUpErrorSetting = libxml_use_internal_errors(true); + + try { + $document = new DOMDocument(); + $document->load($file); + + if (! $document->schemaValidate(__DIR__ . '/../../../../../doctrine-mapping.xsd')) { + throw MappingException::fromLibXmlErrors(libxml_get_errors()); + } + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($backedUpErrorSetting); + } + } + /** * @param mixed $element * diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 0ba7506dfd1..017eacf21e3 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -6,6 +6,7 @@ use BackedEnum; use Doctrine\ORM\Exception\ORMException; +use LibXMLError; use ReflectionException; use ValueError; @@ -17,6 +18,8 @@ use function implode; use function sprintf; +use const PHP_EOL; + /** * A MappingException indicates that something is wrong with the mapping setup. */ @@ -1000,4 +1003,21 @@ public static function invalidEnumValue( $enumType ), 0, $previous); } + + /** + * @param LibXMLError[] $errors + */ + public static function fromLibXmlErrors(array $errors): self + { + $formatter = static function (LibXMLError $error): string { + return sprintf( + 'libxml error: %s in %s at line %d', + $error->message, + $error->file, + $error->line + ); + }; + + return new self(implode(PHP_EOL, array_map($formatter, $errors))); + } } diff --git a/lib/Doctrine/ORM/ORMSetup.php b/lib/Doctrine/ORM/ORMSetup.php index b13d2bd6c0d..3213431d737 100644 --- a/lib/Doctrine/ORM/ORMSetup.php +++ b/lib/Doctrine/ORM/ORMSetup.php @@ -101,10 +101,11 @@ public static function createXMLMetadataConfiguration( array $paths, bool $isDevMode = false, ?string $proxyDir = null, - ?CacheItemPoolInterface $cache = null + ?CacheItemPoolInterface $cache = null, + bool $isXsdValidationEnabled = false ): Configuration { $config = self::createConfiguration($isDevMode, $proxyDir, $cache); - $config->setMetadataDriverImpl(new XmlDriver($paths)); + $config->setMetadataDriverImpl(new XmlDriver($paths, XmlDriver::DEFAULT_FILE_EXTENSION, $isXsdValidationEnabled)); return $config; } diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index 119ca451da6..9ac35db2583 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -1874,3 +1874,11 @@ public static function loadMetadata(ClassMetadataInfo $metadata): void ); } } + +class UserIncorrectAttributes extends User +{ +} + +class UserMissingAttributes extends User +{ +} diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index 96e0deaf63d..a9da33e1e49 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -9,35 +9,34 @@ use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataFactory; use Doctrine\ORM\Mapping\Driver\XmlDriver; +use Doctrine\ORM\Mapping\MappingException; use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Doctrine\Persistence\Mapping\RuntimeReflectionService; use Doctrine\Tests\Models\DDC117\DDC117Translation; use Doctrine\Tests\Models\DDC3293\DDC3293User; use Doctrine\Tests\Models\DDC3293\DDC3293UserPrefixed; use Doctrine\Tests\Models\DDC889\DDC889Class; +use Doctrine\Tests\Models\DDC889\DDC889Entity; +use Doctrine\Tests\Models\DDC889\DDC889SuperClass; use Doctrine\Tests\Models\Generic\SerializationModel; use Doctrine\Tests\Models\GH7141\GH7141Article; use Doctrine\Tests\Models\GH7316\GH7316Article; use Doctrine\Tests\Models\ValueObjects\Name; use Doctrine\Tests\Models\ValueObjects\Person; -use DOMDocument; -use function array_filter; -use function array_map; -use function assert; -use function glob; -use function in_array; -use function is_array; -use function pathinfo; +use function substr_count; use const DIRECTORY_SEPARATOR; -use const PATHINFO_FILENAME; class XmlMappingDriverTest extends AbstractMappingDriverTest { protected function loadDriver(): MappingDriver { - return new XmlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'xml'); + return new XmlDriver( + __DIR__ . DIRECTORY_SEPARATOR . 'xml', + XmlDriver::DEFAULT_FILE_EXTENSION, + true + ); } public function testClassTableInheritanceDiscriminatorMap(): void @@ -163,32 +162,89 @@ public function testInvalidMappingFileException(): void * @dataProvider dataValidSchema * @group DDC-2429 */ - public function testValidateXmlSchema(string $xmlMappingFile): void - { - $xsdSchemaFile = __DIR__ . '/../../../../../doctrine-mapping.xsd'; - $dom = new DOMDocument(); - - $dom->load($xmlMappingFile); - - self::assertTrue($dom->schemaValidate($xsdSchemaFile)); + public function testValidateXmlSchema( + string $class, + string $tableName, + array $fieldNames, + array $associationNames + ): void { + $metadata = $this->createClassMetadata($class); + + $this->assertInstanceOf(ClassMetadata::class, $metadata); + $this->assertEquals($metadata->getTableName(), $tableName); + $this->assertEquals($metadata->getFieldNames(), $fieldNames); + $this->assertEquals($metadata->getAssociationNames(), $associationNames); } /** - * @psalm-return list + * @psalm-return []array{0: class-string, 1: string, 2: list, 3: list} */ public static function dataValidSchema(): array { - $list = glob(__DIR__ . '/xml/*.xml'); - $invalid = ['Doctrine.Tests.Models.DDC889.DDC889Class.dcm']; - assert(is_array($list)); + return [ + [ + User::class, + 'cms_users', + ['name', 'email', 'version', 'id'], + ['address', 'phonenumbers', 'groups'], + ], + [ + DDC889Entity::class, + 'DDC889Entity', + [], + [], + ], + [ + DDC889SuperClass::class, + 'DDC889SuperClass', + ['name'], + [], + ], + ]; + } - $list = array_filter($list, static function (string $item) use ($invalid): bool { - return ! in_array(pathinfo($item, PATHINFO_FILENAME), $invalid, true); - }); + /** + * @param class-string $class + * @param non-empty-array $expectedExceptionOccurrences + * + * @dataProvider dataInvalidSchema + */ + public function testValidateIncorrectXmlSchema(string $class, array $expectedExceptionOccurrences): void + { + try { + $this->createClassMetadata($class); + + $this->fail('XML schema validation should throw a MappingException'); + } catch (MappingException $exception) { + foreach ($expectedExceptionOccurrences as $exceptionContent => $occurrencesCount) { + $this->assertEquals($occurrencesCount, substr_count($exception->getMessage(), $exceptionContent)); + } + } + } - return array_map(static function (string $item) { - return [$item]; - }, $list); + /** + * @return non-empty-list}> + */ + public static function dataInvalidSchema(): array + { + return [ + [ + DDC889Class::class, + ['This element is not expected' => 1], + ], + [ + UserIncorrectAttributes::class, + [ + 'attribute \'field\': The attribute \'field\' is not allowed' => 2, + 'The attribute \'name\' is required but missing' => 2, + 'attribute \'fieldName\': The attribute \'fieldName\' is not allowed' => 1, + ], + ], + [ + UserMissingAttributes::class, + ['The attribute \'name\' is required but missing' => 1], + ], + ]; } /** @@ -225,10 +281,11 @@ public function testManyToManyDefaultOrderByAsc(): void /** * @group DDC-889 */ - public function testinvalidEntityOrMappedSuperClassShouldMentionParentClasses(): void + public function testInvalidEntityOrMappedSuperClassShouldMentionParentClasses(): void { - $this->expectException('Doctrine\Persistence\Mapping\MappingException'); - $this->expectExceptionMessage('Invalid mapping file \'Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml\' for class \'Doctrine\Tests\Models\DDC889\DDC889Class\'.'); + $this->expectException(MappingException::class); + $this->expectExceptionMessage('libxml error: Element \'{http://doctrine-project.org/schemas/orm/doctrine-mapping}class\': This element is not expected.'); + $this->createClassMetadata(DDC889Class::class); } } diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml index ea069f8eb4e..0b8a163c7f8 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml @@ -1,7 +1,7 @@ @@ -9,5 +9,5 @@ - - + + \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Entity.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Entity.dcm.xml index 7fc72bb0ea6..a1de4ea2c40 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Entity.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Entity.dcm.xml @@ -1,10 +1,10 @@ - + diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889SuperClass.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889SuperClass.dcm.xml index c204e8651d5..6d03f391948 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889SuperClass.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889SuperClass.dcm.xml @@ -1,11 +1,11 @@ - + diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserIncorrectAttributes.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserIncorrectAttributes.dcm.xml new file mode 100644 index 00000000000..f486dd9aedd --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserIncorrectAttributes.dcm.xml @@ -0,0 +1,65 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserMissingAttributes.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserMissingAttributes.dcm.xml new file mode 100644 index 00000000000..d736fa080fc --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.UserMissingAttributes.dcm.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + +