Skip to content

Commit

Permalink
[BUGFIX] Fix undefined key warning in cObj->imageLinkWrap
Browse files Browse the repository at this point in the history 
The default configuration for JSWindow in
lib.contentElement doesn't contain the params
property, so after stdWrapping it in
ContentObjectRenderer, it becomes an empty string.

Thus, exploding it needs a fallback to prevent
PHP 8.0 undefined array key warnings.

In addition, first tests have been added for
cObj->imageLinkWrap, which cover this warning. These
tests have also found another warning for
JSwindow.altUrl_noDefaultParams, which is also
fixed now.

Resolves: #96647
Related: #96508
Releases: main, 11.5
Change-Id: I593e42e3be8ebac4e8001275f2da9d635936c79e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73175
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: physikbuddha <r.kaerner@oranto.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
  • Loading branch information
@Physikbuddha @lolli42
Physikbuddha authored and lolli42 committed Feb 7, 2022
1 parent 191002b commit c3fa86f
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 10 deletions.
7 changes: 1 addition & 6 deletions Build/phpstan/phpstan-baseline.neon
View file
Expand Up @@ -3951,7 +3951,7 @@ parameters:
path: ../../typo3/sysext/fluid/Tests/Functional/ViewHelpers/Link/FileViewHelperTest.php

-
message: "#^Parameter \\#1 \\$fileObject of method TYPO3\\\\CMS\\\\Frontend\\\\ContentObject\\\\ContentObjectRenderer\\:\\:setCurrentFile\\(\\) expects TYPO3\\\\CMS\\\\Core\\\\Resource\\\\File, TYPO3\\\\CMS\\\\Core\\\\Resource\\\\FileInterface given\\.$#"
message: "#^Parameter \\#1 \\$fileObject of method TYPO3\\\\CMS\\\\Frontend\\\\ContentObject\\\\ContentObjectRenderer\\:\\:setCurrentFile\\(\\) expects string\\|TYPO3\\\\CMS\\\\Core\\\\Resource\\\\File\\|TYPO3\\\\CMS\\\\Core\\\\Resource\\\\FileReference\\|TYPO3\\\\CMS\\\\Core\\\\Resource\\\\Folder\\|null, TYPO3\\\\CMS\\\\Core\\\\Resource\\\\FileInterface given\\.$#"
count: 1
path: ../../typo3/sysext/fluid_styled_content/Classes/ViewHelpers/Link/ClickEnlargeViewHelper.php

Expand Down Expand Up @@ -4270,11 +4270,6 @@ parameters:
count: 2
path: ../../typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php

-
message: "#^Instanceof between TYPO3\\\\CMS\\\\Core\\\\Resource\\\\File and TYPO3\\\\CMS\\\\Core\\\\Resource\\\\FileReference will always evaluate to false\\.$#"
count: 1
path: ../../typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php

-
message: "#^Method TYPO3\\\\CMS\\\\Frontend\\\\ContentObject\\\\ContentObjectRenderer\\:\\:getFileDataKey\\(\\) should return int\\|string but returns null\\.$#"
count: 1
Expand Down
15 changes: 11 additions & 4 deletions typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
View file
Expand Up @@ -1010,7 +1010,7 @@ public function imageLinkWrap($string, $imageFile, $conf)
if ($conf['JSwindow']) {
$altUrl = $this->stdWrapValue('altUrl', $conf['JSwindow.'] ?? []);
if ($altUrl) {
$url = $altUrl . ($conf['JSwindow.']['altUrl_noDefaultParams'] ? '' : '?file=' . rawurlencode((string)$imageFile) . $params);
$url = $altUrl . (($conf['JSwindow.']['altUrl_noDefaultParams'] ?? false) ? '' : '?file=' . rawurlencode((string)$imageFile) . $params);
}

$processedFile = $file->process(ProcessedFile::CONTEXT_IMAGECROPSCALEMASK, $conf);
Expand All @@ -1027,7 +1027,14 @@ public function imageLinkWrap($string, $imageFile, $conf)
$windowParams = (string)$this->stdWrapValue('params', $conf['JSwindow.'] ?? []);
$windowParams = explode(',', $windowParams);
foreach ($windowParams as $windowParam) {
[$paramKey, $paramValue] = explode('=', $windowParam);
$windowParamParts = explode('=', $windowParam);
$paramKey = $windowParamParts[0];
$paramValue = $windowParamParts[1] ?? null;

if ($paramKey === '') {
continue;
}

if ($paramValue !== '') {
$params[$paramKey] = $paramValue;
} else {
Expand Down Expand Up @@ -1127,7 +1134,7 @@ public function getATagParams($conf)
/**
* Sets the current file object during iterations over files.
*
* @param File $fileObject The file object.
* @param File|FileReference|Folder|string|null $fileObject The file object.
*/
public function setCurrentFile($fileObject)
{
Expand All @@ -1137,7 +1144,7 @@ public function setCurrentFile($fileObject)
/**
* Gets the current file object during iterations over files.
*
* @return File The current file object.
* @return File|FileReference|Folder|string|null The current file object.
*/
public function getCurrentFile()
{
Expand Down
205 changes: 205 additions & 0 deletions typo3/sysext/frontend/Tests/Functional/ContentObject/ContentObjectRendererTest.php
View file
Expand Up @@ -24,6 +24,7 @@
use TYPO3\CMS\Core\Context\Context;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Domain\Repository\PageRepository;
use TYPO3\CMS\Core\Resource\FileReference;
use TYPO3\CMS\Core\Routing\PageArguments;
use TYPO3\CMS\Core\Site\SiteFinder;
use TYPO3\CMS\Core\Tests\Functional\SiteHandling\SiteBasedTestTrait;
Expand Down Expand Up @@ -60,9 +61,12 @@ class ContentObjectRendererTest extends FunctionalTestCase
*/
protected $typoScriptFrontendController;

protected array $pathsToProvideInTestInstance = ['typo3/sysext/frontend/Tests/Functional/Fixtures/Images' => 'fileadmin/user_upload'];

protected function setUp(): void
{
parent::setUp();
$this->setUpBackendUserFromFixture(1);
$this->writeSiteConfiguration(
'test',
$this->buildSiteConfiguration(1, '/'),
Expand Down Expand Up @@ -712,4 +716,205 @@ public function checkIfReturnsExpectedValues(array $configuration, bool $expecte
];
self::assertSame($expected, $this->subject->checkIf($configuration));
}

public function imageLinkWrapWrapsTheContentAsConfiguredDataProvider(): iterable
{
$width = 900;
$height = 600;
$processingWidth = $width . 'm';
$processingHeight = $height . 'm';
$defaultConfiguration = [
'wrap' => '<a href="javascript:close();"> | </a>',
'width' => $processingWidth,
'height' => $processingHeight,
'JSwindow' => '1',
'JSwindow.' => [
'newWindow' => '0',
],
'crop.' => [
'data' => 'file:current:crop',
],
'linkParams.' => [
'ATagParams.' => [
'dataWrap' => 'class="lightbox" rel="lightbox[{field:uid}]"',
],
],
'enable' => true,
];
$imageTag = '<img class="image-embed-item" src="/fileadmin/_processed_/team-t3board10-processed.jpg" width="500" height="300" loading="lazy" alt="" />';
$windowFeatures = 'width=' . $width . ',height=' . $height . ',status=0,menubar=0';

$configurationEnableFalse = $defaultConfiguration;
$configurationEnableFalse['enable'] = false;
yield 'enable => false configuration returns image tag as is.' => [
'content' => $imageTag,
'configuration' => $configurationEnableFalse,
'expected' => [$imageTag => true],
];

yield 'image is wrapped with link tag.' => [
'content' => $imageTag,
'configuration' => $defaultConfiguration,
'expected' => [
'<a href="index.php?eID=tx_cms_showpic&amp;file=1' => true,
$imageTag . '</a>' => true,
'data-window-features="' . $windowFeatures => true,
'data-window-target="thePicture"' => true,
' target="' . 'thePicture' => true,
],
];

$paramsConfiguration = $defaultConfiguration;
$windowFeaturesOverrides = 'width=420,status=1,menubar=1,foo=bar';
$windowFeaturesOverriddenExpected = 'width=420,height=' . $height . ',status=1,menubar=1,foo=bar';
$paramsConfiguration['JSwindow.']['params'] = $windowFeaturesOverrides;
yield 'JSWindow.params overrides windowParams' => [
'content' => $imageTag,
'configuration' => $paramsConfiguration,
'expected' => [
'data-window-features="' . $windowFeaturesOverriddenExpected => true,
],
];

$newWindowConfiguration = $defaultConfiguration;
$newWindowConfiguration['JSwindow.']['newWindow'] = '1';
yield 'data-window-target is not "thePicture" if newWindow = 1 but an md5 hash of the url.' => [
'content' => $imageTag,
'configuration' => $newWindowConfiguration,
'expected' => [
'data-window-target="thePicture' => false,
],
];

$newWindowConfiguration = $defaultConfiguration;
$newWindowConfiguration['JSwindow.']['expand'] = '20,40';
$windowFeaturesExpand = 'width=' . ($width + 20) . ',height=' . ($height + 40) . ',status=0,menubar=0';
yield 'expand increases the window size by its value' => [
'content' => $imageTag,
'configuration' => $newWindowConfiguration,
'expected' => [
'data-window-features="' . $windowFeaturesExpand => true,
],
];

$directImageLinkConfiguration = $defaultConfiguration;
$directImageLinkConfiguration['directImageLink'] = '1';
yield 'Direct image link does not use eID and links directly to the image.' => [
'content' => $imageTag,
'configuration' => $directImageLinkConfiguration,
'expected' => [
'index.php?eID=tx_cms_showpic&amp;file=1' => false,
'<a href="fileadmin/_processed_' => true,
'data-window-url="fileadmin/_processed_' => true,
],
];

// @todo Error: Object of class TYPO3\CMS\Core\Resource\FileReference could not be converted to string
// $altUrlConfiguration = $defaultConfiguration;
// $altUrlConfiguration['JSwindow.']['altUrl'] = '/alternative-url';
// yield 'JSwindow.altUrl forces an alternative url.' => [
// 'content' => $imageTag,
// 'configuration' => $altUrlConfiguration,
// 'expected' => [
// '<a href="/alternative-url' => true,
// 'data-window-url="/alternative-url' => true,
// ],
// ];

$altUrlConfigurationNoDefault = $defaultConfiguration;
$altUrlConfigurationNoDefault['JSwindow.']['altUrl'] = '/alternative-url';
$altUrlConfigurationNoDefault['JSwindow.']['altUrl_noDefaultParams'] = '1';
yield 'JSwindow.altUrl_noDefaultParams removes the default ?file= params' => [
'content' => $imageTag,
'configuration' => $altUrlConfigurationNoDefault,
'expected' => [
'<a href="/alternative-url' => true,
'data-window-url="/alternative-url' => true,
'data-window-url="/alternative-url?file=' => false,
],
];

$targetConfiguration = $defaultConfiguration;
$targetConfiguration['target'] = 'myTarget';
yield 'Setting target overrides the default target "thePicture.' => [
'content' => $imageTag,
'configuration' => $targetConfiguration,
'expected' => [
' target="myTarget"' => true,
'data-window-target="thePicture"' => true,
],
];

$parameters = [
'sample' => '1',
'width' => $processingWidth,
'height' => $processingHeight,
'effects' => 'gamma=1.3 | flip | rotate=180',
'bodyTag' => '<body style="margin:0; background:#fff;">',
'title' => 'My Title',
'wrap' => '<div class="my-wrap">|</div>',
'crop' => '{"default":{"cropArea":{"x":0,"y":0,"width":1,"height":1},"selectedRatio":"NaN","focusArea":null}}',
];
$parameterConfiguration = array_replace($defaultConfiguration, $parameters);
$expectedParameters = $parameters;
$expectedParameters['sample'] = 1;
yield 'Setting one of [width, height, effects, bodyTag, title, wrap, crop, sample] will add them to the parameter list.' => [
'content' => $imageTag,
'configuration' => $parameterConfiguration,
'expected' => [],
'expectedParams' => $expectedParameters,
];

$stdWrapConfiguration = $defaultConfiguration;
$stdWrapConfiguration['stdWrap.'] = [
'append' => 'TEXT',
'append.' => [
'value' => 'appendedString',
],
];
yield 'stdWrap is called upon the whole content.' => [
'content' => $imageTag,
'configuration' => $stdWrapConfiguration,
'expected' => [
'appendedString' => true,
],
];
}

/**
* @dataProvider imageLinkWrapWrapsTheContentAsConfiguredDataProvider
* @test
*/
public function imageLinkWrapWrapsTheContentAsConfigured(string $content, array $configuration, array $expected, array $expectedParams = []): void
{
$GLOBALS['TSFE'] = $this->typoScriptFrontendController;
$this->importCSVDataSet(__DIR__ . '/DataSet/FileReferences.csv');
$fileReferenceData = [
'uid' => 1,
'uid_local' => 1,
'crop' => '{"default":{"cropArea":{"x":0,"y":0,"width":1,"height":1},"selectedRatio":"NaN","focusArea":null}}',
];
$fileReference = new FileReference($fileReferenceData);
$this->subject->setCurrentFile($fileReference);
$result = $this->subject->imageLinkWrap($content, $fileReference, $configuration);

foreach ($expected as $expectedString => $shouldContain) {
if ($shouldContain) {
self::assertStringContainsString($expectedString, $result);
} else {
self::assertStringNotContainsString($expectedString, $result);
}
}

if ($expectedParams !== []) {
preg_match('@href="(.*)"@U', $result, $matches);
self::assertArrayHasKey(1, $matches);
$url = parse_url(html_entity_decode($matches[1]));
parse_str($url['query'], $queryResult);
$base64_string = implode('', $queryResult['parameters']);
$base64_decoded = base64_decode($base64_string);
$jsonDecodedArray = json_decode($base64_decoded, true);
self::assertSame($expectedParams, $jsonDecodedArray);
}
}
}
10 changes: 10 additions & 0 deletions typo3/sysext/frontend/Tests/Functional/ContentObject/DataSet/FileReferences.csv
View file
@@ -0,0 +1,10 @@
"tt_content",,,,,,,,,,,,,,,,,,,,
,"uid","pid","header","image","sorting","deleted","hidden","sys_language_uid","l18n_parent","l10n_source","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","categories",,,,
,297,1,"Regular Element #1",1,256,0,0,0,0,0,0,0,0,0,0,0,,,,
"sys_file",,,,,,,,,,,,,,,,,,,,
,"uid","pid","type","storage","identifier","extension","mime_type","name","sha1","size","creation_date","modification_date","missing","metadata","identifier_hash","folder_hash","last_indexed",,,
,1,0,2,1,"/user_upload/team-t3board10.jpg","jpg","image/jpeg","team-t3board10.jpg","ae6951147687ed1f94f60973fca7ef46e2ba2372",166843,1375080761,1374139442,0,0,"16ba2a587da8ef10dfccbe8b9841bde85afbd2d4","19669f1e02c2f16705ec7587044c66443be70725",0,,,
,,,,,,,,,,,,,,,,,,,,
"sys_file_reference",,,,,,,,,,,,,,,,,,,,
,"uid","pid","title","uid_local","uid_foreign","sys_language_uid","l10n_parent","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","deleted","tablenames","fieldname","sorting_foreign","table_local","description","alternative","link","l10n_diffsource"
,1,1,"T3BOARD",1,297,0,0,0,0,0,0,0,"tt_content","image",2,"sys_file",,,,

0 comments on commit c3fa86f

Please sign in to comment.