IBX-12106: Fixed handling of invalid xml:id values in RichText

.github/workflows/backend-ci.yaml

@@ -60,6 +60,8 @@ jobs:
 
             -   name: Run PHPStan analysis
                 run: composer run-script phpstan
+                env:
+                    PHPSTAN_TURBO: ${{ matrix.php == '7.4' && '0' || '' }}
 
             -   name: Run test suite
                 run: composer run-script --timeout=600 test

composer.json

@@ -13,6 +13,7 @@
         "ext-xsl": "*",
         "symfony/asset": "^5.1",
         "symfony/dependency-injection": "^5.0",
+        "symfony/deprecation-contracts": "^2.5 || ^3.0",
         "symfony/http-kernel": "^5.0",
         "symfony/config": "^5.0",
         "symfony/yaml": "^5.0",

phpstan-baseline.neon

@@ -588,12 +588,6 @@ parameters:
 			count: 1
 			path: src/lib/FieldType/RichText/RichTextStorage.php
 
-		-
-			message: '#^Parameter \#1 \$source of method DOMDocument\:\:loadXML\(\) expects string, array\|bool\|float\|int\|string\|null given\.$#'
-			identifier: argument.type
-			count: 2
-			path: src/lib/FieldType/RichText/RichTextStorage.php
-
 		-
 			message: '#^Method Ibexa\\FieldTypeRichText\\FieldType\\RichText\\RichTextStorage\\Gateway\:\:getContentIds\(\) has parameter \$remoteIds with no value type specified in iterable type array\.$#'
 			identifier: missingType.iterableValue
@@ -630,12 +624,6 @@ parameters:
 			count: 1
 			path: src/lib/FieldType/RichText/RichTextStorage/Gateway.php
 
-		-
-			message: '#^Parameter \#1 \$source of method DOMDocument\:\:loadXML\(\) expects string, array\|bool\|float\|int\|string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/lib/FieldType/RichText/SearchField.php
-
 		-
 			message: '#^Access to an undefined property Ibexa\\Contracts\\Core\\FieldType\\Value\:\:\$xml\.$#'
 			identifier: property.notFound
@@ -666,12 +654,6 @@ parameters:
 			count: 1
 			path: src/lib/FieldType/RichText/Type.php
 
-		-
-			message: '#^Parameter \#1 \$xml of class Ibexa\\FieldTypeRichText\\FieldType\\RichText\\Value constructor expects DOMDocument\|string\|null, array\|bool\|float\|int\|string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/lib/FieldType/RichText/Type.php
-
 		-
 			message: '#^Property Ibexa\\FieldTypeRichText\\FieldType\\RichText\\Value\:\:\$xml \(DOMDocument\) in isset\(\) is not nullable\.$#'
 			identifier: isset.property

src/bundle/Resources/config/fieldtype_services.yaml

@@ -142,6 +142,11 @@ services:
         tags:
             - {name: ibexa.field_type.richtext.converter.input.xhtml5, priority: 10}
 
+    # Note: should run after xsl transformation
+    Ibexa\FieldTypeRichText\RichText\Converter\XmlId:
+        tags:
+            - {name: ibexa.field_type.richtext.converter.input.xhtml5, priority: 60}
+
     # Note: should run after xsl transformation
     Ibexa\FieldTypeRichText\RichText\Converter\LiteralLayoutNestedList:
         tags:

src/bundle/Resources/config/rest.yaml

@@ -7,5 +7,6 @@ services:
     Ibexa\FieldTypeRichText\REST\FieldTypeProcessor\RichTextProcessor:
         arguments:
             - '@Ibexa\FieldTypeRichText\RichText\Converter\Html5Edit'
+            - '@Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface'
         tags:
             - { name: ibexa.rest.field_type.processor, alias: ezrichtext }

src/bundle/Resources/config/settings/fieldtype_external_storages.yaml

@@ -1,6 +1,10 @@
 services:
     Ibexa\FieldTypeRichText\FieldType\RichText\RichTextStorage:
-        arguments: ['@Ibexa\FieldTypeRichText\FieldType\RichText\RichTextStorage\Gateway\DoctrineStorage']
+        arguments:
+            $gateway: '@Ibexa\FieldTypeRichText\FieldType\RichText\RichTextStorage\Gateway\DoctrineStorage'
+            $logger: '@?logger'
+            $domDocumentLoader: '@Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface'
         tags:
             - {name: ibexa.field_type.storage.external.handler, alias: ezrichtext}
+            - {name: monolog.logger, channel: ibexa.richtext}
         public: true

src/bundle/Resources/config/settings/fieldtype_services.yaml

@@ -55,6 +55,12 @@ services:
     Ibexa\FieldTypeRichText\RichText\DOMDocumentFactory:
         public: false
 
+    Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader:
+        tags:
+            - {name: monolog.logger, channel: ibexa.richtext}
+
+    Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface: '@Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader'
+
     Ibexa\FieldTypeRichText\RichText\XMLSanitizer:
         public: false
 

src/bundle/Resources/config/settings/fieldtypes.yaml

@@ -5,5 +5,6 @@ services:
         arguments:
             $textExtractor: '@Ibexa\FieldTypeRichText\RichText\TextExtractor\ShortTextExtractor'
             $inputHandler: '@Ibexa\FieldTypeRichText\RichText\InputHandler'
+            $domDocumentLoader: '@Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface'
         tags:
             - {name: ibexa.field_type, alias: ezrichtext}

src/bundle/Resources/config/settings/indexable_fieldtypes.yaml

@@ -3,5 +3,6 @@ services:
         arguments:
             $shortTextExtractor: '@Ibexa\FieldTypeRichText\RichText\TextExtractor\ShortTextExtractor'
             $fullTextExtractor: '@Ibexa\FieldTypeRichText\RichText\TextExtractor\FullTextExtractor'
+            $domDocumentLoader: '@Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface'
         tags:
             - {name: ibexa.field_type.indexable, alias: ezrichtext}

src/contracts/RichText/DOMDocumentLoaderInterface.php

@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Contracts\FieldTypeRichText\RichText;
+
+use DOMDocument;
+
+/**
+ * Loads already stored RichText XML documents. Never throws, libxml errors are logged instead.
+ */
+interface DOMDocumentLoaderInterface
+{
+    /**
+     * @param array<string, mixed> $logContext
+     */
+    public function loadXML(string $xml, array $logContext = []): DOMDocument;
+}

src/lib/FieldType/RichText/RichTextStorage.php

@@ -8,13 +8,14 @@
 
 namespace Ibexa\FieldTypeRichText\FieldType\RichText;
 
-use DOMDocument;
 use DOMXPath;
 use Ibexa\Contracts\Core\FieldType\GatewayBasedStorage;
 use Ibexa\Contracts\Core\FieldType\StorageGateway;
 use Ibexa\Contracts\Core\Persistence\Content\Field;
 use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
+use Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface;
 use Ibexa\Core\Base\Exceptions\NotFoundException;
+use Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader;
 use Psr\Log\LoggerInterface;
 
 class RichTextStorage extends GatewayBasedStorage
@@ -29,23 +30,38 @@ class RichTextStorage extends GatewayBasedStorage
      */
     protected $gateway;
 
+    private DOMDocumentLoaderInterface $domDocumentLoader;
+
+    public function __construct(
+        StorageGateway $gateway,
+        ?LoggerInterface $logger = null,
+        ?DOMDocumentLoaderInterface $domDocumentLoader = null
+    ) {
+        parent::__construct($gateway);
+        $this->logger = $logger;
+        $this->domDocumentLoader = $domDocumentLoader ?? new DOMDocumentLoader($logger);
+    }
+
     /**
-     * @param \Ibexa\Contracts\Core\FieldType\StorageGateway $gateway
-     * @param \Psr\Log\LoggerInterface|null $logger
+     * @return array<string, int>
      */
-    public function __construct(StorageGateway $gateway, ?LoggerInterface $logger = null)
+    private function getLogContext(VersionInfo $versionInfo, Field $field): array
     {
-        parent::__construct($gateway);
-        $this->logger = $logger;
+        return [
+            'contentId' => $versionInfo->contentInfo->id,
+            'versionNo' => $versionInfo->versionNo,
+            'fieldId' => $field->id,
+        ];
     }
 
     /**
      * @see \Ibexa\Contracts\Core\FieldType\FieldStorage
      */
     public function storeFieldData(VersionInfo $versionInfo, Field $field, array $context)
     {
-        $document = new DOMDocument();
-        $document->loadXML($field->value->data);
+        /** @var string $xmlData */
+        $xmlData = $field->value->data;
+        $document = $this->domDocumentLoader->loadXML($xmlData, $this->getLogContext($versionInfo, $field));
 
         $xpath = new DOMXPath($document);
         $xpath->registerNamespace('docbook', 'http://docbook.org/ns/docbook');
@@ -135,8 +151,9 @@ public function storeFieldData(VersionInfo $versionInfo, Field $field, array $co
      */
     public function getFieldData(VersionInfo $versionInfo, Field $field, array $context)
     {
-        $document = new DOMDocument();
-        $document->loadXML($field->value->data);
+        /** @var string $xmlData */
+        $xmlData = $field->value->data;
+        $document = $this->domDocumentLoader->loadXML($xmlData, $this->getLogContext($versionInfo, $field));
 
         $xpath = new DOMXPath($document);
         $xpath->registerNamespace('docbook', 'http://docbook.org/ns/docbook');

src/lib/FieldType/RichText/SearchField.php

@@ -8,12 +8,13 @@
 
 namespace Ibexa\FieldTypeRichText\FieldType\RichText;
 
-use DOMDocument;
 use Ibexa\Contracts\Core\FieldType\Indexable;
 use Ibexa\Contracts\Core\Persistence\Content\Field;
 use Ibexa\Contracts\Core\Persistence\Content\Type\FieldDefinition;
 use Ibexa\Contracts\Core\Search;
+use Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface;
 use Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractorInterface;
+use Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader;
 
 /**
  * Indexable definition for RichText field type.
@@ -24,12 +25,16 @@ class SearchField implements Indexable
 
     private TextExtractorInterface $fullTextExtractor;
 
+    private DOMDocumentLoaderInterface $domDocumentLoader;
+
     public function __construct(
         TextExtractorInterface $shortTextExtractor,
-        TextExtractorInterface $fullTextExtractor
+        TextExtractorInterface $fullTextExtractor,
+        ?DOMDocumentLoaderInterface $domDocumentLoader = null
     ) {
         $this->shortTextExtractor = $shortTextExtractor;
         $this->fullTextExtractor = $fullTextExtractor;
+        $this->domDocumentLoader = $domDocumentLoader ?? new DOMDocumentLoader();
     }
 
     /**
@@ -42,8 +47,13 @@ public function __construct(
      */
     public function getIndexData(Field $field, FieldDefinition $fieldDefinition)
     {
-        $document = new DOMDocument();
-        $document->loadXML($field->value->data);
+        /** @var string $xmlData */
+        $xmlData = $field->value->data;
+        $document = $this->domDocumentLoader->loadXML($xmlData, [
+            'fieldId' => $field->id,
+            'versionNo' => $field->versionNo,
+            'fieldDefinitionIdentifier' => $fieldDefinition->identifier,
+        ]);
 
         return [
             new Search\Field(

src/lib/FieldType/RichText/Type.php

@@ -12,12 +12,14 @@
 use Ibexa\Contracts\Core\FieldType\Value as SPIValue;
 use Ibexa\Contracts\Core\Persistence\Content\FieldValue;
 use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition;
+use Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface;
 use Ibexa\Contracts\FieldTypeRichText\RichText\InputHandlerInterface;
 use Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractorInterface;
 use Ibexa\Core\Base\Exceptions\InvalidArgumentType;
 use Ibexa\Core\FieldType\FieldType;
 use Ibexa\Core\FieldType\ValidationError;
 use Ibexa\Core\FieldType\Value as BaseValue;
+use Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader;
 use JMS\TranslationBundle\Model\Message;
 use JMS\TranslationBundle\Translation\TranslationContainerInterface;
 use RuntimeException;
@@ -34,12 +36,16 @@ class Type extends FieldType implements TranslationContainerInterface
 
     private TextExtractorInterface $textExtractor;
 
+    private DOMDocumentLoaderInterface $domDocumentLoader;
+
     public function __construct(
         InputHandlerInterface $inputHandler,
-        TextExtractorInterface $textExtractor
+        TextExtractorInterface $textExtractor,
+        ?DOMDocumentLoaderInterface $domDocumentLoader = null
     ) {
         $this->inputHandler = $inputHandler;
         $this->textExtractor = $textExtractor;
+        $this->domDocumentLoader = $domDocumentLoader ?? new DOMDocumentLoader();
     }
 
     /**
@@ -224,7 +230,11 @@ public function toHash(SPIValue $value)
      */
     public function fromPersistenceValue(FieldValue $fieldValue)
     {
-        return new Value($fieldValue->data);
+        if (!is_string($fieldValue->data) || $fieldValue->data === '') {
+            return new Value();
+        }
+
+        return new Value($this->domDocumentLoader->loadXML($fieldValue->data));
     }
 
     /**

src/lib/FieldType/RichText/Value.php

@@ -9,6 +9,7 @@
 namespace Ibexa\FieldTypeRichText\FieldType\RichText;
 
 use DOMDocument;
+use Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface;
 use Ibexa\Core\FieldType\Value as BaseValue;
 
 /**
@@ -31,16 +32,30 @@ class Value extends BaseValue
     /**
      * Initializes a new RichText Value object with $xmlDoc in.
      *
-     * @param \DOMDocument|string $xml
+     * @param \DOMDocument|null $xml passing a string is deprecated since 4.6.33 and will not be supported in 6.0
      */
     public function __construct($xml = null)
     {
         if ($xml instanceof DOMDocument) {
             $this->xml = $xml;
-        } else {
-            $this->xml = new DOMDocument();
-            $this->xml->loadXML($xml === null ? self::EMPTY_VALUE : $xml);
+
+            return;
+        }
+
+        if ($xml !== null) {
+            trigger_deprecation(
+                'ibexa/fieldtype-richtext',
+                '4.6.33',
+                'Passing string as $xml argument of %s() is deprecated and will not be supported in 6.0. '
+                . 'Pass \DOMDocument instead, e.g. loaded with %s service.',
+                __METHOD__,
+                DOMDocumentLoaderInterface::class
+            );
         }
+
+        $this->xml = new DOMDocument();
+        /** @phpstan-ignore nullCoalesce.variable (deprecated string $xml is not part of the declared signature) */
+        $this->xml->loadXML($xml ?? self::EMPTY_VALUE);
     }
 
     /**

src/lib/REST/FieldTypeProcessor/RichTextProcessor.php

@@ -8,9 +8,10 @@
 
 namespace Ibexa\FieldTypeRichText\REST\FieldTypeProcessor;
 
-use DOMDocument;
 use Ibexa\Contracts\FieldTypeRichText\RichText\Converter;
+use Ibexa\Contracts\FieldTypeRichText\RichText\DOMDocumentLoaderInterface;
 use Ibexa\Contracts\Rest\FieldTypeProcessor;
+use Ibexa\FieldTypeRichText\RichText\DOMDocumentLoader;
 
 class RichTextProcessor extends FieldTypeProcessor
 {
@@ -19,18 +20,22 @@ class RichTextProcessor extends FieldTypeProcessor
      */
     protected $docbookToXhtml5EditConverter;
 
-    public function __construct(Converter $docbookToXhtml5EditConverter)
-    {
+    private DOMDocumentLoaderInterface $domDocumentLoader;
+
+    public function __construct(
+        Converter $docbookToXhtml5EditConverter,
+        ?DOMDocumentLoaderInterface $domDocumentLoader = null
+    ) {
         $this->docbookToXhtml5EditConverter = $docbookToXhtml5EditConverter;
+        $this->domDocumentLoader = $domDocumentLoader ?? new DOMDocumentLoader();
     }
 
     /**
      * {@inheritdoc}
      */
     public function postProcessValueHash($outgoingValueHash)
     {
-        $document = new DOMDocument();
-        $document->loadXML($outgoingValueHash['xml']);
+        $document = $this->domDocumentLoader->loadXML($outgoingValueHash['xml']);
 
         $outgoingValueHash['xhtml5edit'] = $this->docbookToXhtml5EditConverter
             ->convert($document)