Skip to content

Commit

Permalink
Merge pull request #6728 from greg0ire/validate_xml_against_xsd
Browse files Browse the repository at this point in the history 
Validate XML mapping against XSD file
  • Loading branch information
@greg0ire
greg0ire authored Apr 25, 2022
2 parents a552df6 + ab3a255 commit f4585b9
Show file tree
Hide file tree
Showing 11 changed files with 267 additions and 44 deletions.
1 change: 1 addition & 0 deletions composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
51 changes: 50 additions & 1 deletion lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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(
Expand All @@ -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);
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
*
Expand Down
20 changes: 20 additions & 0 deletions lib/Doctrine/ORM/Mapping/MappingException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use BackedEnum;
use Doctrine\ORM\Exception\ORMException;
use LibXMLError;
use ReflectionException;
use ValueError;

Expand All @@ -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.
*/
Expand Down Expand Up @@ -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)));
}
}
5 changes: 3 additions & 2 deletions lib/Doctrine/ORM/ORMSetup.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 8 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1874,3 +1874,11 @@ public static function loadMetadata(ClassMetadataInfo $metadata): void
);
}
}

class UserIncorrectAttributes extends User
{
}

class UserMissingAttributes extends User
{
}
119 changes: 88 additions & 31 deletions tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<array{string}>
* @psalm-return []array{0: class-string, 1: string, 2: list<string>, 3: list<string>}
*/
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<string, int> $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<array{0: class-string, 1: non-empty-array<string, int>}>
*/
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],
],
];
}

/**
Expand Down Expand Up @@ -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);
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Class.dcm.xml
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">

<class name="Doctrine\Tests\Models\DDC889\DDC889Class">
<id name="id" type="integer" column="id">
<generator strategy="AUTO"/>
</id>
</class>
</doctrine-mapping>

</doctrine-mapping>
6 changes: 3 additions & 3 deletions tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889Entity.dcm.xml
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">

<entity name="Doctrine\Tests\Models\DDC889\DDC889Entity">
</entity>

</doctrine-mapping>
6 changes: 3 additions & 3 deletions tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.DDC889.DDC889SuperClass.dcm.xml
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">

<mapped-superclass name="Doctrine\Tests\Models\DDC889\DDC889SuperClass">
<field name="name" column="name" type="string"/>
</mapped-superclass>

</doctrine-mapping>
Loading

0 comments on commit f4585b9

Please sign in to comment.