From 56fa57cf398d79d9e2a0dceb143e253974d6663a Mon Sep 17 00:00:00 2001 From: Beatrycze Volk Date: Mon, 20 Nov 2023 16:06:09 +0100 Subject: [PATCH] Fix error: Direct use of $GLOBALS Superglobal detected --- Classes/Command/BaseCommand.php | 2 +- Classes/Command/HarvestCommand.php | 2 +- Classes/Command/IndexCommand.php | 4 +-- Classes/Command/ReindexCommand.php | 2 +- Classes/Common/AbstractDocument.php | 21 +++++++---- Classes/Common/Indexer.php | 2 +- Classes/Controller/AbstractController.php | 36 ++++++++++++------- Classes/Controller/DocumentController.php | 2 +- .../Domain/Repository/DocumentRepository.php | 4 +-- .../DocumentTypeFunctionProvider.php | 6 ++-- Classes/Hooks/DataHandler.php | 4 +-- Tests/Functional/Common/MetsDocumentTest.php | 2 +- Tests/Functional/Common/SolrIndexingTest.php | 2 +- 13 files changed, 54 insertions(+), 35 deletions(-) diff --git a/Classes/Command/BaseCommand.php b/Classes/Command/BaseCommand.php index 25f2b5e476..ae073d268e 100644 --- a/Classes/Command/BaseCommand.php +++ b/Classes/Command/BaseCommand.php @@ -289,7 +289,7 @@ protected function getParentDocumentUidForSaving(Document $document): int if ($doc !== null && !empty($doc->parentHref)) { // find document object by record_id of parent - $parent = AbstractDocument::getInstance($doc->parentHref, ['storagePid' => $this->storagePid]); + $parent = AbstractDocument::getInstance($doc->parentHref, 0, ['storagePid' => $this->storagePid]); if ($parent->recordId) { $parentDocument = $this->documentRepository->findOneByRecordId($parent->recordId); diff --git a/Classes/Command/HarvestCommand.php b/Classes/Command/HarvestCommand.php index b3e565538e..39b96a7d49 100644 --- a/Classes/Command/HarvestCommand.php +++ b/Classes/Command/HarvestCommand.php @@ -226,7 +226,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $docLocation = $baseLocation . http_build_query($params); // ...index the document... $document = null; - $doc = AbstractDocument::getInstance($docLocation, ['storagePid' => $this->storagePid], true); + $doc = AbstractDocument::getInstance($docLocation, 0, ['storagePid' => $this->storagePid], true); if ($doc === null) { $io->warning('WARNING: Document "' . $docLocation . '" could not be loaded. Skip to next document.'); diff --git a/Classes/Command/IndexCommand.php b/Classes/Command/IndexCommand.php index c8dbd4ce65..18c77abf8f 100644 --- a/Classes/Command/IndexCommand.php +++ b/Classes/Command/IndexCommand.php @@ -162,11 +162,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int $io->error('ERROR: Document with UID "' . $input->getOption('doc') . '" could not be found on PID ' . $this->storagePid . ' .'); exit(1); } else { - $doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $this->storagePid], true); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $this->storagePid], true); } } else if (GeneralUtility::isValidUrl($input->getOption('doc'))) { - $doc = AbstractDocument::getInstance($input->getOption('doc'), ['storagePid' => $this->storagePid], true); + $doc = AbstractDocument::getInstance($input->getOption('doc'), 0, ['storagePid' => $this->storagePid], true); $document = $this->getDocumentFromUrl($doc, $input->getOption('doc')); } diff --git a/Classes/Command/ReindexCommand.php b/Classes/Command/ReindexCommand.php index 5ebdc92136..d762c2249c 100644 --- a/Classes/Command/ReindexCommand.php +++ b/Classes/Command/ReindexCommand.php @@ -162,7 +162,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } foreach ($documents as $id => $document) { - $doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $this->storagePid], true); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $this->storagePid], true); if ($doc === null) { $io->warning('WARNING: Document "' . $document->getLocation() . '" could not be loaded. Skip to next document.'); diff --git a/Classes/Common/AbstractDocument.php b/Classes/Common/AbstractDocument.php index 7a81833c79..3e91d59f8e 100644 --- a/Classes/Common/AbstractDocument.php +++ b/Classes/Common/AbstractDocument.php @@ -71,6 +71,12 @@ abstract class AbstractDocument */ protected int $cPid = 0; + /** + * @access protected + * @var int This holds the page ID for the requests + */ + protected int $pageId = 0; + /** * @access public * @static @@ -551,12 +557,13 @@ abstract public function getAllFiles(): array; * @static * * @param string $location The URL of XML file or the IRI of the IIIF resource + * @param int $pageId * @param array $settings * @param bool $forceReload Force reloading the document instead of returning the cached instance * * @return AbstractDocument|null Instance of this class, either MetsDocument or IiifManifest */ - public static function &getInstance(string $location, array $settings = [], bool $forceReload = false) + public static function &getInstance(string $location, int $pageId = 0, array $settings = [], bool $forceReload = false) { // Create new instance depending on format (METS or IIIF) ... $documentFormat = null; @@ -607,11 +614,11 @@ public static function &getInstance(string $location, array $settings = [], bool // Sanitize input. $pid = max((int) $settings['storagePid'], 0); if ($documentFormat == 'METS') { - $instance = new MetsDocument($pid, $location, $xml, $settings); + $instance = new MetsDocument($pid, $location, $pageId, $xml, $settings); } elseif ($documentFormat == 'IIIF') { // TODO: Parameter $preloadedDocument of class Kitodo\Dlf\Common\IiifManifest constructor expects SimpleXMLElement|Ubl\Iiif\Presentation\Common\Model\Resources\IiifResourceInterface, Ubl\Iiif\Presentation\Common\Model\AbstractIiifEntity|null given. // @phpstan-ignore-next-line - $instance = new IiifManifest($pid, $location, $iiif); + $instance = new IiifManifest($pid, $location, $pageId, $iiif); } if ($instance) { @@ -1171,7 +1178,7 @@ protected function magicGetRootId(): int if ($this->parentId) { // TODO: Parameter $location of static method AbstractDocument::getInstance() expects string, int|int<1, max> given. // @phpstan-ignore-next-line - $parent = self::getInstance($this->parentId, ['storagePid' => $this->pid]); + $parent = self::getInstance($this->parentId, $this->pageId, ['storagePid' => $this->pid]); $this->rootId = $parent->rootId; } $this->rootIdLoaded = true; @@ -1219,14 +1226,16 @@ protected function _setCPid(int $value): void * * @param int $pid If > 0, then only document with this PID gets loaded * @param string $location The location URL of the XML file to parse + * @param int $pageId * @param \SimpleXMLElement|IiifResourceInterface $preloadedDocument Either null or the \SimpleXMLElement * or IiifResourceInterface that has been loaded to determine the basic document format. * * @return void */ - protected function __construct(int $pid, string $location, $preloadedDocument, array $settings = []) + protected function __construct(int $pid, string $location, int $pageId, $preloadedDocument, array $settings = []) { $this->pid = $pid; + $this->pageId = $pageId; $this->setPreloadedDocument($preloadedDocument); $this->init($location, $settings); $this->establishRecordId($pid); @@ -1422,7 +1431,7 @@ public function toArray($uriBuilder, array $config = []) // Configure @action URL for form. $file['url'] = $uriBuilder ->reset() - ->setTargetPageUid($GLOBALS['TSFE']->id) + ->setTargetPageUid($this->pageId) ->setCreateAbsoluteUri($forceAbsoluteUrl) ->setArguments( [ diff --git a/Classes/Common/Indexer.php b/Classes/Common/Indexer.php index 59a652db7a..321e7def1d 100644 --- a/Classes/Common/Indexer.php +++ b/Classes/Common/Indexer.php @@ -107,7 +107,7 @@ public static function add(Document $document, DocumentRepository $documentRepos $parent = $documentRepository->findByUid($parentId); if ($parent) { // get XML document of parent - $doc = AbstractDocument::getInstance($parent->getLocation(), ['storagePid' => $parent->getPid()], true); + $doc = AbstractDocument::getInstance($parent->getLocation(), 0, ['storagePid' => $parent->getPid()], true); if ($doc !== null) { $parent->setCurrentDocument($doc); $success = self::add($parent, $documentRepository); diff --git a/Classes/Controller/AbstractController.php b/Classes/Controller/AbstractController.php index e7db9f0ee1..ddef84ffe4 100644 --- a/Classes/Controller/AbstractController.php +++ b/Classes/Controller/AbstractController.php @@ -15,6 +15,7 @@ use Kitodo\Dlf\Common\Helper; use Kitodo\Dlf\Domain\Model\Document; use Kitodo\Dlf\Domain\Repository\DocumentRepository; +use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerAwareTrait; use TYPO3\CMS\Core\Configuration\ExtensionConfiguration; @@ -121,15 +122,17 @@ protected function loadDocument(int $documentId = 0): void $documentId = $this->requestData['id']; } + $pageId = $this->getRequest()->getAttribute('routing')->getPageId(); + // Try to get document format from database if (!empty($documentId)) { $doc = null; if (MathUtility::canBeInterpretedAsInteger($documentId)) { - $doc = $this->getDocumentByUid($documentId); + $doc = $this->getDocumentByUid($documentId, $pageId); } elseif (GeneralUtility::isValidUrl($documentId)) { - $doc = $this->getDocumentByUrl($documentId); + $doc = $this->getDocumentByUrl($documentId, $pageId); } if ($this->document !== null && $doc !== null) { @@ -141,7 +144,7 @@ protected function loadDocument(int $documentId = 0): void $this->document = $this->documentRepository->findOneByRecordId($this->requestData['recordId']); if ($this->document !== null) { - $doc = AbstractDocument::getInstance($this->document->getLocation(), $this->settings, true); + $doc = AbstractDocument::getInstance($this->document->getLocation(), $pageId, $this->settings, true); if ($doc !== null) { $this->document->setCurrentDocument($doc); } else { @@ -165,7 +168,7 @@ protected function loadDocument(int $documentId = 0): void protected function configureProxyUrl(string &$url): void { $this->uriBuilder->reset() - ->setTargetPageUid($GLOBALS['TSFE']->id) + ->setTargetPageUid($this->getRequest()->getAttribute('routing')->getPageId()) ->setCreateAbsoluteUri(!empty($this->settings['forceAbsoluteUrl'])) ->setArguments( [ @@ -434,22 +437,28 @@ protected function buildSimplePagination(PaginationInterface $pagination, Pagina ]; } + protected function getRequest(): ServerRequestInterface + { + return $GLOBALS['TYPO3_REQUEST']; + } + /** * Get document from repository by uid. * * @access private * * @param int $documentId The document's UID + * @param int $pageId * * @return AbstractDocument */ - private function getDocumentByUid(int $documentId) + private function getDocumentByUid(int $documentId, int $pageId) { $doc = null; + // find document from repository by uid $this->document = $this->documentRepository->findOneByIdAndSettings($documentId); - if ($this->document) { - $doc = AbstractDocument::getInstance($this->document->getLocation(), $this->settings, true); + $doc = AbstractDocument::getInstance($this->document->getLocation(), $pageId, $this->settings, true); } else { $this->logger->error('Invalid UID "' . $documentId . '" or PID "' . $this->settings['storagePid'] . '" for document loading'); } @@ -463,12 +472,13 @@ private function getDocumentByUid(int $documentId) * @access private * * @param string $documentId The document's URL + * @param int $pageId * * @return AbstractDocument */ - private function getDocumentByUrl(string $documentId) + private function getDocumentByUrl(string $documentId, int $pageId) { - $doc = AbstractDocument::getInstance($documentId, $this->settings, true); + $doc = AbstractDocument::getInstance($documentId, $pageId, $this->settings, true); if ($doc !== null) { if ($doc->recordId) { @@ -484,13 +494,13 @@ private function getDocumentByUrl(string $documentId) // Make sure configuration PID is set when applicable if ($doc->cPid == 0) { - $doc->cPid = max((int) $this->settings['storagePid'], 0); + $doc->cPid = max(intval($this->settings['storagePid']), 0); } $this->document->setLocation($documentId); - } else { - $this->logger->error('Invalid location given "' . $documentId . '" for document loading'); - } + } else { + $this->logger->error('Invalid location given "' . $documentId . '" for document loading'); + } return $doc; } diff --git a/Classes/Controller/DocumentController.php b/Classes/Controller/DocumentController.php index 8d4c9ab784..61489dabaf 100644 --- a/Classes/Controller/DocumentController.php +++ b/Classes/Controller/DocumentController.php @@ -119,7 +119,7 @@ protected function getUrlTemplate() $make = function ($page, $double, $pageGrid) { $result = $this->uriBuilder->reset() - ->setTargetPageUid($this->configurationManager->getContentObject()->data['pid']) + ->setTargetPageUid($this->getRequest()->getAttribute('routing')->getPageId()) ->setCreateAbsoluteUri(!empty($this->settings['forceAbsoluteUrl']) ? true : false) ->setArguments( [ diff --git a/Classes/Domain/Repository/DocumentRepository.php b/Classes/Domain/Repository/DocumentRepository.php index e0307b63e5..795e848595 100644 --- a/Classes/Domain/Repository/DocumentRepository.php +++ b/Classes/Domain/Repository/DocumentRepository.php @@ -76,7 +76,7 @@ public function findOneByParameters($parameters) } else if (isset($parameters['location']) && GeneralUtility::isValidUrl($parameters['location'])) { - $doc = AbstractDocument::getInstance($parameters['location'], [], true); + $doc = AbstractDocument::getInstance($parameters['location'], 0, [], true); if ($doc->recordId) { $document = $this->findOneByRecordId($doc->recordId); @@ -91,7 +91,7 @@ public function findOneByParameters($parameters) } if ($document !== null && $doc === null) { - $doc = AbstractDocument::getInstance($document->getLocation(), [], true); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, [], true); } if ($doc !== null) { diff --git a/Classes/ExpressionLanguage/DocumentTypeFunctionProvider.php b/Classes/ExpressionLanguage/DocumentTypeFunctionProvider.php index b9b374fa54..a7a9af5ae7 100644 --- a/Classes/ExpressionLanguage/DocumentTypeFunctionProvider.php +++ b/Classes/ExpressionLanguage/DocumentTypeFunctionProvider.php @@ -179,13 +179,13 @@ protected function loadDocument(array $requestData, int $pid): void // find document from repository by uid $this->document = $this->documentRepository->findOneByIdAndSettings((int) $requestData['id'], ['storagePid' => $pid]); if ($this->document) { - $doc = AbstractDocument::getInstance($this->document->getLocation(), ['storagePid' => $pid], true); + $doc = AbstractDocument::getInstance($this->document->getLocation(), 0, ['storagePid' => $pid], true); } else { $this->logger->error('Invalid UID "' . $requestData['id'] . '" or PID "' . $pid . '" for document loading'); } } else if (GeneralUtility::isValidUrl($requestData['id'])) { - $doc = AbstractDocument::getInstance($requestData['id'], ['storagePid' => $pid], true); + $doc = AbstractDocument::getInstance($requestData['id'], 0, ['storagePid' => $pid], true); if ($doc !== null) { if ($doc->recordId) { @@ -212,7 +212,7 @@ protected function loadDocument(array $requestData, int $pid): void $this->document = $this->documentRepository->findOneByRecordId($requestData['recordId']); if ($this->document !== null) { - $doc = AbstractDocument::getInstance($this->document->getLocation(), ['storagePid' => $pid], true); + $doc = AbstractDocument::getInstance($this->document->getLocation(), 0, ['storagePid' => $pid], true); if ($doc !== null) { $this->document->setCurrentDocument($doc); } else { diff --git a/Classes/Hooks/DataHandler.php b/Classes/Hooks/DataHandler.php index ad646b3220..b43ae3ca26 100644 --- a/Classes/Hooks/DataHandler.php +++ b/Classes/Hooks/DataHandler.php @@ -258,7 +258,7 @@ public function processDatamap_afterDatabaseOperations(string $status, string $t } else { // Reindex document. $document = $this->getDocumentRepository()->findByUid((int) $id); - $doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $document->getPid()], true); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $document->getPid()], true); if ($document !== null && $doc !== null) { $document->setCurrentDocument($doc); Indexer::add($document, $this->getDocumentRepository()); @@ -340,7 +340,7 @@ public function processCmdmap_postProcess(string $command, string $table, $id): case 'undelete': // Reindex document. $document = $this->getDocumentRepository()->findByUid((int) $id); - $doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $document->getPid()], true); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $document->getPid()], true); if ($document !== null && $doc !== null) { $document->setCurrentDocument($doc); Indexer::add($document, $this->getDocumentRepository()); diff --git a/Tests/Functional/Common/MetsDocumentTest.php b/Tests/Functional/Common/MetsDocumentTest.php index 49b3b8fa70..dffaea6351 100644 --- a/Tests/Functional/Common/MetsDocumentTest.php +++ b/Tests/Functional/Common/MetsDocumentTest.php @@ -29,7 +29,7 @@ public function setUp(): void protected function doc(string $file) { $url = 'http://web:8001/Tests/Fixtures/MetsDocument/' . $file; - $doc = AbstractDocument::getInstance($url, ['useExternalApisForMetadata' => 0]); + $doc = AbstractDocument::getInstance($url, 0, ['useExternalApisForMetadata' => 0]); self::assertNotNull($doc); return $doc; } diff --git a/Tests/Functional/Common/SolrIndexingTest.php b/Tests/Functional/Common/SolrIndexingTest.php index 221fe63473..0b55bd8493 100644 --- a/Tests/Functional/Common/SolrIndexingTest.php +++ b/Tests/Functional/Common/SolrIndexingTest.php @@ -77,7 +77,7 @@ public function canIndexAndSearchDocument() $document->setSolrcore($core->model->getUid()); $this->persistenceManager->persistAll(); - $doc = AbstractDocument::getInstance($document->getLocation(), ['useExternalApisForMetadata' => 0]); + $doc = AbstractDocument::getInstance($document->getLocation(), 0, ['useExternalApisForMetadata' => 0]); $document->setCurrentDocument($doc); $indexingSuccessful = Indexer::add($document, $this->documentRepository);