diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 16f5ee8ae8..07ecb2f8d9 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -12,6 +12,7 @@ parameters: message: "#^Cannot call method (fetchOne|fetchColumn|fetchAllAssociative|fetchAssociative|fetchAllKeyValue)\\(\\) on Doctrine\\\\DBAL\\\\ForwardCompatibility\\\\Result\\|int\\|string\\.$#" paths: - src/* + - tests/* paths: - src - tests diff --git a/src/bundle/IO/Resources/config/io.yml b/src/bundle/IO/Resources/config/io.yml index 3a2d242262..cd694a0b2a 100644 --- a/src/bundle/IO/Resources/config/io.yml +++ b/src/bundle/IO/Resources/config/io.yml @@ -102,18 +102,13 @@ services: arguments: - '@Ibexa\Core\IO\IOConfigProvider' - # Base service for flysystem metadata handler - ibexa.core.io.metadata_handler.flysystem: - abstract: true - class: Ibexa\Core\IO\IOMetadataHandler\Flysystem - arguments: - - ~ - # Default flysystem metadata handler ibexa.core.io.metadata_handler.flysystem.default: class: Ibexa\Core\IO\IOMetadataHandler\Flysystem arguments: - '@ibexa.core.io.flysystem.default_filesystem' + tags: + - { name: monolog.logger, channel: ibexa.core.io } # Base service for flysystem binarydata handler ibexa.core.io.binarydata_handler.flysystem: diff --git a/src/lib/IO/IOMetadataHandler/Flysystem.php b/src/lib/IO/IOMetadataHandler/Flysystem.php index f70249d43b..f294f5b74e 100644 --- a/src/lib/IO/IOMetadataHandler/Flysystem.php +++ b/src/lib/IO/IOMetadataHandler/Flysystem.php @@ -12,16 +12,28 @@ use Ibexa\Core\IO\Exception\BinaryFileNotFoundException; use Ibexa\Core\IO\Exception\IOException; use Ibexa\Core\IO\IOMetadataHandler; +use League\Flysystem\CorruptedPathDetected; use League\Flysystem\FilesystemException; use League\Flysystem\FilesystemOperator; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; -class Flysystem implements IOMetadataHandler +class Flysystem implements IOMetadataHandler, LoggerAwareInterface { + private LoggerInterface $logger; + private FilesystemOperator $filesystem; - public function __construct(FilesystemOperator $filesystem) + public function __construct(FilesystemOperator $filesystem, ?LoggerInterface $logger = null) { $this->filesystem = $filesystem; + $this->logger = $logger ?? new NullLogger(); + } + + public function setLogger(LoggerInterface $logger): void + { + $this->logger = $logger; } /** @@ -56,6 +68,13 @@ public function exists($spiBinaryFileId): bool { try { return $this->filesystem->fileExists($spiBinaryFileId); + } catch (CorruptedPathDetected $e) { + $this->logger->error( + sprintf('Binary file with ID="%s" does not exist: %s', $spiBinaryFileId, $e->getMessage()), + ['exception' => $e], + ); + + return false; } catch (FilesystemException $e) { throw new IOException( "Unable to check if file '$spiBinaryFileId' exists: {$e->getMessage()}", diff --git a/src/lib/Resources/settings/io.yml b/src/lib/Resources/settings/io.yml index 023c72d8fa..ad98eb6b8c 100644 --- a/src/lib/Resources/settings/io.yml +++ b/src/lib/Resources/settings/io.yml @@ -29,6 +29,8 @@ services: class: Ibexa\Core\IO\IOMetadataHandler\Flysystem arguments: - '@ibexa.core.io.flysystem.default_filesystem' + tags: + - { name: monolog.logger, channel: ibexa.core.io } # binarydata handlers Ibexa\Core\IO\IOBinarydataHandler\SiteAccessDependentBinaryDataHandler: diff --git a/tests/integration/Core/Repository/FieldType/ImageIntegrationTest.php b/tests/integration/Core/Repository/FieldType/ImageIntegrationTest.php index 8fa2e3e74e..1f3ba37631 100644 --- a/tests/integration/Core/Repository/FieldType/ImageIntegrationTest.php +++ b/tests/integration/Core/Repository/FieldType/ImageIntegrationTest.php @@ -6,10 +6,16 @@ */ namespace Ibexa\Tests\Integration\Core\Repository\FieldType; +use Doctrine\DBAL\ParameterType; +use DOMDocument; +use DOMElement; use Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException; use Ibexa\Contracts\Core\Repository\Values\Content\Content; use Ibexa\Contracts\Core\Repository\Values\Content\Field; +use Ibexa\Core\FieldType\Image\IO\Legacy; use Ibexa\Core\FieldType\Image\Value as ImageValue; +use Ibexa\Core\IO\IOServiceInterface; +use Ibexa\Core\Persistence\Legacy\Content\Gateway; /** * Integration test for use field type. @@ -719,6 +725,136 @@ public function testRemovingDraftRemovesOldImage(): void ); } + /** + * @throws \Doctrine\DBAL\Driver\Exception + * @throws \Doctrine\DBAL\Exception + * @throws \ErrorException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\Exception + */ + public function testDeleteImageWithCorruptedName(): void + { + $ioService = $this->getSetupFactory()->getServiceContainer()->get(Legacy::class); + $repository = $this->getRepository(); + $contentService = $repository->getContentService(); + + self::assertInstanceOf(IOServiceInterface::class, $ioService); + + $content = $this->publishNewImage( + __METHOD__, + new ImageValue( + [ + 'inputUri' => __DIR__ . '/_fixtures/image.jpg', + 'fileName' => 'image.jpg', + 'fileSize' => filesize(__DIR__ . '/_fixtures/image.jpg'), + 'alternativeText' => 'Alternative', + ] + ), + [2] + ); + + $imageFieldDefinition = $content->getContentType()->getFieldDefinition('image'); + self::assertNotNull($imageFieldDefinition); + + // sanity check + $this->assertImageExists(true, $ioService, $content); + + $record = $this->fetchXML( + $content->id, + $content->getVersionInfo()->versionNo, + $imageFieldDefinition->id + ); + + $document = $this->corruptImageFieldXML($record); + + $this->updateXML( + $content->id, + $content->getVersionInfo()->versionNo, + $imageFieldDefinition->id, + $document + ); + + // reload content to get the field value with the corrupted path + $content = $contentService->loadContent($content->id); + $this->assertImageExists(false, $ioService, $content); + + $contentService->deleteContent($content->getVersionInfo()->getContentInfo()); + + // Expect no League\Flysystem\CorruptedPathDetected thrown + } + + /** + * @return array + * + * @throws \Doctrine\DBAL\Exception + * @throws \ErrorException + * @throws \Doctrine\DBAL\Driver\Exception + */ + private function fetchXML(int $contentId, int $versionNo, int $fieldDefinitionId): array + { + $connection = $this->getRawDatabaseConnection(); + + $query = $connection->createQueryBuilder(); + $query + ->select('data_text') + ->from(Gateway::CONTENT_FIELD_TABLE) + ->andWhere('contentclassattribute_id = :contentclassattribute_id') + ->andWhere('version = :version') + ->andWhere('contentobject_id = :contentobject_id') + ->setParameter('contentclassattribute_id', $fieldDefinitionId, ParameterType::INTEGER) + ->setParameter('version', $versionNo, ParameterType::INTEGER) + ->setParameter('contentobject_id', $contentId, ParameterType::INTEGER); + + $result = $query->execute()->fetchAssociative(); + self::assertNotFalse($result); + + return $result; + } + + /** + * @param array $row + */ + private function corruptImageFieldXML(array $row): DOMDocument + { + $corruptedChar = '­'; + + $document = new DOMDocument('1.0', 'utf-8'); + $document->loadXML($row['data_text']); + $elements = $document->getElementsByTagName('ezimage'); + $element = $elements->item(0); + self::assertInstanceOf(DOMElement::class, $element); + $element->setAttribute('filename', $element->getAttribute('filename') . $corruptedChar); + $element->setAttribute('url', $element->getAttribute('url') . $corruptedChar); + + return $document; + } + + /** + * @throws \Doctrine\DBAL\Exception + * @throws \ErrorException + */ + private function updateXML( + int $contentId, + int $versionNo, + int $fieldDefinitionId, + DOMDocument $document + ): void { + $connection = $this->getRawDatabaseConnection(); + + $query = $connection->createQueryBuilder(); + $query + ->update(Gateway::CONTENT_FIELD_TABLE) + ->set('data_text', ':data_text') + ->setParameter('data_text', $document->saveXML(), ParameterType::STRING) + ->andWhere('contentclassattribute_id = :contentclassattribute_id') + ->andWhere('version = :version') + ->andWhere('contentobject_id = :contentobject_id') + ->setParameter('contentclassattribute_id', $fieldDefinitionId, ParameterType::INTEGER) + ->setParameter('version', $versionNo, ParameterType::INTEGER) + ->setParameter('contentobject_id', $contentId, ParameterType::INTEGER); + + $query->execute(); + } + /** * @throws \Ibexa\Contracts\Core\Repository\Exceptions\ForbiddenException * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException @@ -780,6 +916,16 @@ private function getImageURI(Content $content): string { return $content->getFieldValue('image')->uri; } + + private function assertImageExists(bool $expectExists, IOServiceInterface $ioService, Content $content): void + { + $imageField = $content->getField('image'); + self::assertNotNull($imageField, 'Image field not found'); + + /** @var \Ibexa\Core\FieldType\Image\Value $imageFieldValue */ + $imageFieldValue = $imageField->value; + self::assertSame($expectExists, $ioService->exists($imageFieldValue->id)); + } } class_alias(ImageIntegrationTest::class, 'eZ\Publish\API\Repository\Tests\FieldType\ImageIntegrationTest');