Skip to content

Commit

Permalink
Link::getImageLink - array $name parameter
Browse files Browse the repository at this point in the history
There are some third party modules that pass array instead of string as
$name paraterer to Link::getImageLink method. Most likely this is caused
by loading Product object in all-lang context, and then passing
$link_rewrite property.

Previously, thirty bees suvived such calls, and silently cast array to
string. Image name then looked like this: 1-Niara_cart/Array.jpg

Since 1.4, PHP raises error, because the array is then passed to method
that has name parameter annotated as string.

This commit fixes this error, and instead raises warning.
  • Loading branch information
getdatakick committed Jan 31, 2023
1 parent 6e49bf0 commit 809f16c
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 1 deletion.
35 changes: 35 additions & 0 deletions classes/Link.php
Expand Up @@ -362,6 +362,19 @@ public function getImageLink($name, $ids, $type = null, $format = null, $highDpi
$ids = (string)$ids;
$context = Context::getContext();

if (is_null($name)) {
$name = $ids;
}
if (! is_string($name)) {
$callPoint = Tools::getCallPoint([Link::class]);
$errorMessage = 'Link::getImageLink(): parameter $name has invalid type. ';
$errorMessage .= 'Expected string, got ' . gettype($name) . '. ';
$errorMessage .= 'This will raise error in future version of thirty bees. ';
$errorMessage .= 'Called from: ' . $callPoint['class'] . '::' . $callPoint['function'] . '() in ' . $callPoint['file'] . ':' . $callPoint['line'];
trigger_error($errorMessage, E_USER_WARNING);
$name = static::resolveName($name, $ids);
}

if (!$format) {
$format = ImageManager::webpSupport() ? 'webp' : 'jpg';
}
Expand Down Expand Up @@ -1206,4 +1219,26 @@ protected function getProductImageUri(int $imageId, string $formattedType, bool

return false;
}

/**
* @param mixed $name
* @param string $default
*
* @return string
*/
protected static function resolveName($name, $default)
{
if (is_array($name)) {
$languageId = Context::getContext()->language->id;
if (isset($name[$languageId])) {
return (string)$name[$languageId];
}
foreach ($name as $value) {
if (is_string($value)) {
return $value;
}
}
}
return $default;
}
}
2 changes: 1 addition & 1 deletion classes/Tools.php
Expand Up @@ -1169,7 +1169,7 @@ public static function getCallPoint($ignoreClassNames = [])
'class' => $class,
'function' => $trace['function'],
'line' => $prev['line'],
'file' => $prev['file'],
'file' => ErrorUtils::getRelativeFile($prev['file']),
];
}
$prev = $trace;
Expand Down
108 changes: 108 additions & 0 deletions tests/Integration/LinkTest.php
@@ -0,0 +1,108 @@
<?php

namespace Tests\Integration;

use Codeception\Test\Unit;
use Configuration;
use Link;
use PrestaShopException;
use Product;
use Tests\Support\UnitTester;
use function PHPUnit\Framework\assertEquals;

class LinkTest extends Unit
{
/**
* @var UnitTester
*/
protected UnitTester $tester;

/**
* @var bool
*/
protected $raised = false;

protected function getImageLinkData()
{
return [
['1/name.jpg', 1, 'name', 1, null],
['img/p/1/1.jpg', 0, 'name', 1, null],
['1-Niara_home/name.jpg', 1, 'name', 1, 'home'],
['img/p/1/1-Niara_home.jpg', 0, 'name', 1, 'home'],
['img/p/en-default-Niara_home.jpg', 1, 'name', 111111, 'home'],
['img/p/en-default-Niara_home.jpg', 0, 'name', 111111, 'home'],
['1-Niara_home/name.jpg', 1, 'name', 1, 'home'],
['img/p/1/1-Niara_home.jpg', 0, 'name', 1, 'home'],
];
}

/**
* @throws PrestaShopException
*
* @dataProvider getImageLinkData
*
*/
public function testImageLink($expected, $rewrite, $name, $ids, $type)
{
try {
Configuration::updateValue('PS_REWRITING_SETTINGS', $rewrite);
$link = new Link('http://', 'http://');
assertEquals($expected, static::getRelativeUrl($link->getImageLink($name, $ids, $type)));
} finally {
Configuration::updateValue('PS_REWRITING_SETTINGS', 0);
}
}

/**
* @return void
* @throws PrestaShopException
*/
public function testImageLinkForSingleLangProduct()
{
try {
Configuration::updateValue('PS_REWRITING_SETTINGS', 1);
$link = new Link('http://', 'http://');
$product = new Product(1, false, 1);
$imageId = $product->getCoverWs();
assertEquals('1-Niara_cart/candle.jpg', static::getRelativeUrl($link->getImageLink($product->link_rewrite, $imageId, 'cart')));
assertEquals('1-Niara_cart/candle.jpg', static::getRelativeUrl($link->getImageLink($product->link_rewrite, $imageId, 'cart')));
} finally {
Configuration::updateValue('PS_REWRITING_SETTINGS', 0);
}
}

/**
* @return void
* @throws PrestaShopException
*/
public function testImageLinkForMultiLangProduct()
{
$this->raised = false;
$previous = set_error_handler(function() {
$this->raised = true;
return true;
});
try {
Configuration::updateValue('PS_REWRITING_SETTINGS', 1);
$link = new Link('http://', 'http://');
$product = new Product(1);
$imageId = $product->getCoverWs();
assertEquals('1-Niara_cart/candle.jpg', static::getRelativeUrl($link->getImageLink($product->link_rewrite, $imageId, 'cart')));
assertEquals(true, $this->raised, "Error should have been raised");
} finally {
Configuration::updateValue('PS_REWRITING_SETTINGS', 0);
set_error_handler($previous);
}
}

/**
* @param string $url
*
* @return string
*/
protected static function getRelativeUrl($url)
{
return str_replace(trim(_PS_BASE_URL_, '/') . '/', '', $url);
}

}

0 comments on commit 809f16c

Please sign in to comment.