diff --git a/.github/workflows/backend-ci.yaml b/.github/workflows/backend-ci.yaml index 2afd9664..6c04871b 100644 --- a/.github/workflows/backend-ci.yaml +++ b/.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 diff --git a/composer.json b/composer.json index 7ea249c4..df7f2589 100644 --- a/composer.json +++ b/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", diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9f5d3b5d..3b09f28d 100644 --- a/phpstan-baseline.neon +++ b/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 diff --git a/src/bundle/Resources/config/fieldtype_services.yaml b/src/bundle/Resources/config/fieldtype_services.yaml index 90c1f461..7290a247 100644 --- a/src/bundle/Resources/config/fieldtype_services.yaml +++ b/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: diff --git a/src/bundle/Resources/config/rest.yaml b/src/bundle/Resources/config/rest.yaml index 7aae8148..e2f202f6 100644 --- a/src/bundle/Resources/config/rest.yaml +++ b/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 } diff --git a/src/bundle/Resources/config/settings/fieldtype_external_storages.yaml b/src/bundle/Resources/config/settings/fieldtype_external_storages.yaml index cb55655d..963ba301 100644 --- a/src/bundle/Resources/config/settings/fieldtype_external_storages.yaml +++ b/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 diff --git a/src/bundle/Resources/config/settings/fieldtype_services.yaml b/src/bundle/Resources/config/settings/fieldtype_services.yaml index c1e8d251..28190398 100644 --- a/src/bundle/Resources/config/settings/fieldtype_services.yaml +++ b/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 diff --git a/src/bundle/Resources/config/settings/fieldtypes.yaml b/src/bundle/Resources/config/settings/fieldtypes.yaml index 831b6ba8..681715f2 100644 --- a/src/bundle/Resources/config/settings/fieldtypes.yaml +++ b/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} diff --git a/src/bundle/Resources/config/settings/indexable_fieldtypes.yaml b/src/bundle/Resources/config/settings/indexable_fieldtypes.yaml index 1b466e54..f3789527 100644 --- a/src/bundle/Resources/config/settings/indexable_fieldtypes.yaml +++ b/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} diff --git a/src/contracts/RichText/DOMDocumentLoaderInterface.php b/src/contracts/RichText/DOMDocumentLoaderInterface.php new file mode 100644 index 00000000..dadf802f --- /dev/null +++ b/src/contracts/RichText/DOMDocumentLoaderInterface.php @@ -0,0 +1,22 @@ + $logContext + */ + public function loadXML(string $xml, array $logContext = []): DOMDocument; +} diff --git a/src/lib/FieldType/RichText/RichTextStorage.php b/src/lib/FieldType/RichText/RichTextStorage.php index 3782c2c8..ae0f1c5b 100644 --- a/src/lib/FieldType/RichText/RichTextStorage.php +++ b/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,14 +30,28 @@ 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 */ - 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, + ]; } /** @@ -44,8 +59,9 @@ public function __construct(StorageGateway $gateway, ?LoggerInterface $logger = */ 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'); diff --git a/src/lib/FieldType/RichText/SearchField.php b/src/lib/FieldType/RichText/SearchField.php index 6fb847c1..3624d525 100644 --- a/src/lib/FieldType/RichText/SearchField.php +++ b/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( diff --git a/src/lib/FieldType/RichText/Type.php b/src/lib/FieldType/RichText/Type.php index 38b3afba..a4b1898c 100644 --- a/src/lib/FieldType/RichText/Type.php +++ b/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)); } /** diff --git a/src/lib/FieldType/RichText/Value.php b/src/lib/FieldType/RichText/Value.php index 60e458b7..2d40c300 100644 --- a/src/lib/FieldType/RichText/Value.php +++ b/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); } /** diff --git a/src/lib/REST/FieldTypeProcessor/RichTextProcessor.php b/src/lib/REST/FieldTypeProcessor/RichTextProcessor.php index 28436850..67039a9b 100644 --- a/src/lib/REST/FieldTypeProcessor/RichTextProcessor.php +++ b/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,9 +20,14 @@ 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(); } /** @@ -29,8 +35,7 @@ public function __construct(Converter $docbookToXhtml5EditConverter) */ public function postProcessValueHash($outgoingValueHash) { - $document = new DOMDocument(); - $document->loadXML($outgoingValueHash['xml']); + $document = $this->domDocumentLoader->loadXML($outgoingValueHash['xml']); $outgoingValueHash['xhtml5edit'] = $this->docbookToXhtml5EditConverter ->convert($document) diff --git a/src/lib/RichText/Converter/XmlId.php b/src/lib/RichText/Converter/XmlId.php new file mode 100644 index 00000000..cdec5e90 --- /dev/null +++ b/src/lib/RichText/Converter/XmlId.php @@ -0,0 +1,130 @@ +registerNamespace('xlink', self::XLINK_NAMESPACE); + + $sanitizedFragmentMap = $this->sanitizeIds($xpath); + if (!empty($sanitizedFragmentMap)) { + $this->rewriteInternalLinks($xpath, $sanitizedFragmentMap); + } + + return $document; + } + + /** + * @return array map of original href fragment => sanitized href fragment + */ + private function sanitizeIds(DOMXPath $xpath): array + { + $elements = $xpath->query('//*[@xml:id]') ?: []; + + $usedIds = []; + /** @var \DOMElement $element */ + foreach ($elements as $element) { + $id = $element->getAttribute('xml:id'); + if ($this->isValidNCName($id)) { + $usedIds[$id] = true; + } + } + + $sanitizedFragmentMap = []; + /** @var \DOMElement $element */ + foreach ($elements as $element) { + $id = $element->getAttribute('xml:id'); + if ($this->isValidNCName($id)) { + continue; + } + + if ($id === '') { + $element->removeAttribute('xml:id'); + continue; + } + + $sanitizedId = $this->buildUniqueId($this->sanitizeId($id), $usedIds); + $usedIds[$sanitizedId] = true; + if (!isset($sanitizedFragmentMap['#' . $id])) { + $sanitizedFragmentMap['#' . $id] = '#' . $sanitizedId; + } + $element->setAttribute('xml:id', $sanitizedId); + } + + return $sanitizedFragmentMap; + } + + /** + * @param array $sanitizedFragmentMap + */ + private function rewriteInternalLinks(DOMXPath $xpath, array $sanitizedFragmentMap): void + { + $links = $xpath->query('//*[starts-with(@xlink:href, "#")]') ?: []; + /** @var \DOMElement $link */ + foreach ($links as $link) { + $href = $link->getAttribute('xlink:href'); + if (isset($sanitizedFragmentMap[$href])) { + $link->setAttribute('xlink:href', $sanitizedFragmentMap[$href]); + } + } + } + + private function isValidNCName(string $id): bool + { + return preg_match('/^[' . self::NCNAME_START_CHAR . '][' . self::NCNAME_CHAR . ']*\z/u', $id) === 1; + } + + private function sanitizeId(string $id): string + { + $sanitizedId = (string)preg_replace('/[^' . self::NCNAME_CHAR . ']/u', '_', $id); + if (preg_match('/^[' . self::NCNAME_START_CHAR . ']/u', $sanitizedId) !== 1) { + $sanitizedId = '_' . $sanitizedId; + } + + return $sanitizedId; + } + + /** + * @param array $usedIds + */ + private function buildUniqueId(string $id, array $usedIds): string + { + $uniqueId = $id; + $suffix = 1; + while (isset($usedIds[$uniqueId])) { + $uniqueId = $id . '_' . $suffix++; + } + + return $uniqueId; + } +} diff --git a/src/lib/RichText/DOMDocumentLoader.php b/src/lib/RichText/DOMDocumentLoader.php new file mode 100644 index 00000000..d4018c39 --- /dev/null +++ b/src/lib/RichText/DOMDocumentLoader.php @@ -0,0 +1,55 @@ +logger = $logger ?? new NullLogger(); + } + + public function loadXML(string $xml, array $logContext = []): DOMDocument + { + $document = new DOMDocument(); + $useInternalErrors = libxml_use_internal_errors(true); + try { + $document->loadXML($xml); + $errors = libxml_get_errors(); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($useInternalErrors); + } + + if (!empty($errors)) { + $this->logger->warning( + 'RichText XML document loaded with libxml errors', + $logContext + ['errors' => array_map([$this, 'formatError'], $errors)] + ); + } + + return $document; + } + + private function formatError(LibXMLError $error): string + { + return sprintf('[line %d] %s', $error->line, trim($error->message)); + } +} diff --git a/tests/integration/Repository/RichTextFieldTypeIntegrationTest.php b/tests/integration/Repository/RichTextFieldTypeIntegrationTest.php index ee120e59..0275fe33 100644 --- a/tests/integration/Repository/RichTextFieldTypeIntegrationTest.php +++ b/tests/integration/Repository/RichTextFieldTypeIntegrationTest.php @@ -476,10 +476,18 @@ public function providerForTestIsEmptyValue() return [ [new RichTextValue()], - [new RichTextValue($xml)], + [new RichTextValue($this->createDocumentFromString($xml))], ]; } + private function createDocumentFromString(string $xml): DOMDocument + { + $document = new DOMDocument(); + $document->loadXML($xml); + + return $document; + } + public function providerForTestIsNotEmptyValue() { $xml = <<getValidCreationFieldData(), ], - [new RichTextValue($xml)], - [new RichTextValue($xml2)], + [new RichTextValue($this->createDocumentFromString($xml))], + [new RichTextValue($this->createDocumentFromString($xml2))], ]; } diff --git a/tests/lib/FieldType/RichText/RichTextStorageTest.php b/tests/lib/FieldType/RichText/RichTextStorageTest.php index 871eebe6..8b5cba40 100644 --- a/tests/lib/FieldType/RichText/RichTextStorageTest.php +++ b/tests/lib/FieldType/RichText/RichTextStorageTest.php @@ -9,6 +9,7 @@ namespace Ibexa\Tests\FieldTypeRichText\FieldType\RichText; use Ibexa\Contracts\Core\FieldType\StorageGateway; +use Ibexa\Contracts\Core\Persistence\Content\ContentInfo; use Ibexa\Contracts\Core\Persistence\Content\Field; use Ibexa\Contracts\Core\Persistence\Content\FieldValue; use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; @@ -88,7 +89,7 @@ public function testGetFieldData($xmlString, $updatedXmlString, $linkIds, $linkU ->method('error') ->withConsecutive($errorMessages); - $versionInfo = new VersionInfo(); + $versionInfo = new VersionInfo(['contentInfo' => new ContentInfo(['id' => 1])]); $value = new FieldValue(['data' => $xmlString]); $field = new Field(['value' => $value]); @@ -186,7 +187,7 @@ public function testStoreFieldData( $contentIds, $isUpdated ): void { - $versionInfo = new VersionInfo(['versionNo' => 24]); + $versionInfo = new VersionInfo(['versionNo' => 24, 'contentInfo' => new ContentInfo(['id' => 1])]); $value = new FieldValue(['data' => $xmlString]); $field = new Field(['id' => 42, 'value' => $value]); $gateway = $this->getGatewayMock(); @@ -338,7 +339,7 @@ public function testStoreFieldDataThrowsNotFoundException( ->willReturn($linkMap['id']); } - $versionInfo = new VersionInfo(); + $versionInfo = new VersionInfo(['contentInfo' => new ContentInfo(['id' => 1])]); $value = new FieldValue(['data' => $xmlString]); $field = new Field(['value' => $value]); @@ -352,7 +353,7 @@ public function testStoreFieldDataThrowsNotFoundException( public function testDeleteFieldData(): void { - $versionInfo = new VersionInfo(['versionNo' => 42]); + $versionInfo = new VersionInfo(['versionNo' => 42, 'contentInfo' => new ContentInfo(['id' => 1])]); $fieldIds = [12, 23]; $gateway = $this->getGatewayMock(); $storage = $this->getPartlyMockedStorage($gateway); diff --git a/tests/lib/FieldType/RichText/ValueTest.php b/tests/lib/FieldType/RichText/ValueTest.php new file mode 100644 index 00000000..4fd2e110 --- /dev/null +++ b/tests/lib/FieldType/RichText/ValueTest.php @@ -0,0 +1,64 @@ + +
Lorem ipsum
'; + + public function testCreateFromDOMDocument(): void + { + $document = new DOMDocument(); + $document->loadXML(self::XML); + + $value = new Value($document); + + self::assertSame($document, $value->xml); + } + + public function testCreateEmptyValue(): void + { + $value = new Value(); + + self::assertSame(Value::EMPTY_VALUE, trim((string)$value)); + } + + public function testCreateFromStringIsDeprecated(): void + { + $deprecations = []; + set_error_handler( + static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, + E_USER_DEPRECATED + ); + + try { + /** @phpstan-ignore argument.type (deliberately passing the deprecated string) */ + $value = new Value(self::XML); + } finally { + restore_error_handler(); + } + + self::assertNotNull($value->xml->documentElement); + self::assertSame('section', $value->xml->documentElement->localName); + self::assertCount(1, $deprecations); + self::assertStringContainsString('Passing string as $xml argument', $deprecations[0]); + } +} diff --git a/tests/lib/FieldType/RichTextTest.php b/tests/lib/FieldType/RichTextTest.php index af0dc10d..97949276 100644 --- a/tests/lib/FieldType/RichTextTest.php +++ b/tests/lib/FieldType/RichTextTest.php @@ -8,7 +8,9 @@ namespace Ibexa\Tests\FieldTypeRichText\FieldType; +use DOMDocument; use Exception; +use Ibexa\Contracts\Core\Persistence\Content\FieldValue; use Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException as ApiInvalidArgumentException; use Ibexa\Contracts\Core\Repository\Values\Content\Relation; use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition as APIFieldDefinition; @@ -294,7 +296,7 @@ public function providerForTestValidate() public function testValidate($xmlString, array $expectedValidationErrors) { $fieldType = $this->getFieldType(); - $value = new Value($xmlString); + $value = new Value($this->createDocument($xmlString)); /** @var \Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition|\PHPUnit\Framework\MockObject\MockObject $fieldDefinitionMock */ $fieldDefinitionMock = $this->createMock(APIFieldDefinition::class); @@ -323,6 +325,32 @@ public function testToPersistenceValue() self::assertSame($xmlString, $fieldValue->data); } + public function testFromPersistenceValueWithInvalidXmlIdDoesNotEmitWarning(): void + { + $xmlString = ' +
Lorem ipsum
'; + + $value = $this->getFieldType()->fromPersistenceValue(new FieldValue(['data' => $xmlString])); + + self::assertNotNull($value->xml->documentElement); + self::assertStringContainsString('xml:id="227"', (string)$value); + } + + public function testFromPersistenceValueWithNullData(): void + { + $value = $this->getFieldType()->fromPersistenceValue(new FieldValue(['data' => null])); + + self::assertSame(Value::EMPTY_VALUE, trim((string)$value)); + } + + private function createDocument(string $xmlString): DOMDocument + { + $document = new DOMDocument(); + $document->loadXML($xmlString); + + return $document; + } + /** * @covers \Ibexa\FieldTypeRichText\FieldType\RichText\Type::getName * @@ -330,7 +358,7 @@ public function testToPersistenceValue() */ public function testGetName($xmlString, $expectedName) { - $value = new Value($xmlString); + $value = new Value($this->createDocument($xmlString)); $fieldType = $this->getFieldType(); $this->assertEquals( diff --git a/tests/lib/RichText/Converter/XmlIdTest.php b/tests/lib/RichText/Converter/XmlIdTest.php new file mode 100644 index 00000000..8a784b1e --- /dev/null +++ b/tests/lib/RichText/Converter/XmlIdTest.php @@ -0,0 +1,137 @@ +'; + + /** + * @return array> + */ + public static function providerConvert(): array + { + // Paragraph-only cases, expressed as input xml:id list => expected xml:id list (null = no attribute) + $idCases = [ + 'numeric id gets prefixed' => [['227'], ['_227']], + 'space in id replaced' => [['foo bar'], ['foo_bar']], + 'colon in id replaced' => [['foo:bar'], ['foo_bar']], + 'sanitized ids colliding with each other get deduplicated' => [ + ['foo bar', 'foo:bar'], + ['foo_bar', 'foo_bar_1'], + ], + 'sanitized id colliding with existing valid id gets deduplicated' => [ + ['foo_bar', 'foo bar'], + ['foo_bar', 'foo_bar_1'], + ], + 'sanitized id colliding with valid id appearing later gets deduplicated' => [ + ['foo bar', 'foo_bar'], + ['foo_bar_1', 'foo_bar'], + ], + 'leading digit gets prefixed' => [['1lipsum'], ['_1lipsum']], + 'valid ids are untouched' => [['lipsum_id1', 'good.id-1'], ['lipsum_id1', 'good.id-1']], + 'valid UTF-8 id is untouched' => [['zażółć'], ['zażółć']], + 'invalid symbol characters replaced' => [['a×b!'], ['a_b_']], + 'empty id gets removed' => [[''], [null]], + 'document without ids is untouched' => [[null], [null]], + ]; + + $cases = []; + foreach ($idCases as $name => [$inputIds, $expectedIds]) { + $cases[$name] = [self::buildParagraphs($inputIds), self::buildParagraphs($expectedIds)]; + } + + $externalLinks = 'Lorem' + . 'ipsum'; + $danglingFragment = 'Lorem ipsum'; + + return $cases + [ + 'internal link fragment follows the sanitized anchor id' => [ + self::buildAnchorWithInternalLink('227'), + self::buildAnchorWithInternalLink('_227'), + ], + 'external link fragments are untouched' => [ + self::buildParagraphs(['227']) . $externalLinks, + self::buildParagraphs(['_227']) . $externalLinks, + ], + 'dangling internal fragment is untouched' => [$danglingFragment, $danglingFragment], + ]; + } + + /** + * @param array $ids + */ + private static function buildParagraphs(array $ids): string + { + $xml = ''; + foreach ($ids as $index => $id) { + $attribute = $id === null ? '' : sprintf(' xml:id="%s"', $id); + $xml .= sprintf('Lorem ipsum %d', $attribute, $index); + } + + return $xml; + } + + private static function buildAnchorWithInternalLink(string $id): string + { + return sprintf( + 'Loremipsum', + $id, + $id + ); + } + + /** + * @dataProvider providerConvert + */ + public function testConvert(string $input, string $output): void + { + $inputDocument = $this->createDocument(self::SECTION_OPEN_TAG . $input . ''); + + $converter = new XmlId(); + + $outputDocument = $converter->convert($inputDocument); + + $expectedOutputDocument = $this->createDocument(self::SECTION_OPEN_TAG . $output . ''); + + self::assertEquals($expectedOutputDocument, $outputDocument); + self::assertLoadsWithoutLibXmlErrors((string)$outputDocument->saveXML()); + + self::assertEquals( + $expectedOutputDocument, + $converter->convert($this->createDocument(self::SECTION_OPEN_TAG . $output . '')) + ); + } + + private function createDocument(string $xml): DOMDocument + { + return (new DOMDocumentLoader())->loadXML($xml); + } + + private static function assertLoadsWithoutLibXmlErrors(string $xml): void + { + $document = new DOMDocument(); + $useInternalErrors = libxml_use_internal_errors(true); + try { + $document->loadXML($xml); + self::assertSame([], libxml_get_errors(), 'Sanitized document should load without libxml errors'); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($useInternalErrors); + } + } +} diff --git a/tests/lib/RichText/Converter/Xslt/Xhtml5ToDocbookTest.php b/tests/lib/RichText/Converter/Xslt/Xhtml5ToDocbookTest.php index 5d70ceed..59599486 100644 --- a/tests/lib/RichText/Converter/Xslt/Xhtml5ToDocbookTest.php +++ b/tests/lib/RichText/Converter/Xslt/Xhtml5ToDocbookTest.php @@ -11,6 +11,7 @@ use Ibexa\FieldTypeRichText\RichText\Converter\Aggregate; use Ibexa\FieldTypeRichText\RichText\Converter\LiteralLayoutNestedList; use Ibexa\FieldTypeRichText\RichText\Converter\ProgramListing; +use Ibexa\FieldTypeRichText\RichText\Converter\XmlId; use Ibexa\FieldTypeRichText\RichText\Converter\Xslt; /** @@ -111,6 +112,7 @@ protected function getConverter() $this->getConversionTransformationStylesheet(), $this->getCustomConversionTransformationStylesheets() ), + new XmlId(), new LiteralLayoutNestedList(), ] ); diff --git a/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.docbook.xml b/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.docbook.xml new file mode 100644 index 00000000..be73bc44 --- /dev/null +++ b/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.docbook.xml @@ -0,0 +1,17 @@ + +
+ Paragraph with numeric id pasted from external HTML. + Paragraph with a space in the id. + Paragraph with a valid id. + + Jump to the anchor below + + + External link, fragment must stay + + Anchored content. +
diff --git a/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.xhtml5.edit.xml b/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.xhtml5.edit.xml new file mode 100644 index 00000000..6b4477c9 --- /dev/null +++ b/tests/lib/RichText/Converter/Xslt/_fixtures/xhtml5/edit/lossy/011-invalidXmlId.xhtml5.edit.xml @@ -0,0 +1,13 @@ + +
+

Paragraph with numeric id pasted from external HTML.

+

Paragraph with a space in the id.

+

Paragraph with a valid id.

+

+ Jump to the anchor below +

+

+ External link, fragment must stay +

+

Anchored content.

+
diff --git a/tests/lib/RichText/DOMDocumentLoaderTest.php b/tests/lib/RichText/DOMDocumentLoaderTest.php new file mode 100644 index 00000000..7a253c99 --- /dev/null +++ b/tests/lib/RichText/DOMDocumentLoaderTest.php @@ -0,0 +1,85 @@ +logger = $this->createMock(LoggerInterface::class); + $this->loader = new DOMDocumentLoader($this->logger); + } + + public function testLoadValidXML(): void + { + $this->logger->expects(self::never())->method('warning'); + + $document = $this->loader->loadXML( + '
Lorem
' + ); + + self::assertNotNull($document->documentElement); + self::assertSame('section', $document->documentElement->localName); + } + + public function testLoadXMLWithInvalidXmlIdLogsWarningWithContext(): void + { + $this->logger + ->expects(self::once()) + ->method('warning') + ->with( + 'RichText XML document loaded with libxml errors', + self::callback(static function (array $context): bool { + return $context['contentId'] === 42 + && $context['fieldId'] === 7 + && count($context['errors']) === 1 + && strpos($context['errors'][0], '227') !== false; + }) + ); + + $document = $this->loader->loadXML( + '
Lorem
', + ['contentId' => 42, 'fieldId' => 7] + ); + + self::assertNotNull($document->documentElement); + self::assertStringContainsString('xml:id="227"', (string)$document->saveXML()); + } + + public function testLoadUnparseableXMLReturnsEmptyDocumentAndLogs(): void + { + $this->logger->expects(self::once())->method('warning'); + + $document = $this->loader->loadXML('
unclosed
'); + + self::assertNull($document->documentElement); + } + + public function testLoadXMLRestoresLibxmlErrorHandling(): void + { + $previous = libxml_use_internal_errors(false); + + $this->loader->loadXML('