From 6a8a103ce27d872e5687402a50ba6e594ae82194 Mon Sep 17 00:00:00 2001 From: Alexander Aleman Date: Fri, 21 Feb 2020 10:39:36 +0100 Subject: [PATCH 1/6] Do not escape custom attributes --- .../Magento/Catalog/view/frontend/templates/product/image.phtml | 2 +- .../view/frontend/templates/product/image_with_borders.phtml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml index 5a31f3d125c81..ef2ba856fad84 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml @@ -7,7 +7,7 @@ escapeHtml($block->getCustomAttributes()) ?> + getCustomAttributes() ?> src="escapeUrl($block->getImageUrl()) ?>" width="escapeHtmlAttr($block->getWidth()) ?>" height="escapeHtmlAttr($block->getHeight()) ?>" diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml index 33f7620f1a1f5..8d0cd0294c48e 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml @@ -11,7 +11,7 @@ escapeHtmlAttr($block->getCustomAttributes()) ?> + getCustomAttributes() ?> src="escapeUrl($block->getImageUrl()) ?>" max-width="escapeHtmlAttr($block->getWidth()) ?>" max-height="escapeHtmlAttr($block->getHeight()) ?>" From 4d3a05d101723432d8610160552b563c25cc3703 Mon Sep 17 00:00:00 2001 From: Alexander Aleman Date: Fri, 21 Feb 2020 10:45:36 +0100 Subject: [PATCH 2/6] Custom attributes can not be escaped in the current form --- .../Magento/Catalog/view/frontend/templates/product/image.phtml | 2 +- .../view/frontend/templates/product/image_with_borders.phtml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml index ef2ba856fad84..2a251b00521d8 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml @@ -7,7 +7,7 @@ getCustomAttributes() ?> + getCustomAttributes() ?> src="escapeUrl($block->getImageUrl()) ?>" width="escapeHtmlAttr($block->getWidth()) ?>" height="escapeHtmlAttr($block->getHeight()) ?>" diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml index 8d0cd0294c48e..32d5ec8b288a9 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml @@ -11,7 +11,7 @@ getCustomAttributes() ?> + getCustomAttributes() ?> src="escapeUrl($block->getImageUrl()) ?>" max-width="escapeHtmlAttr($block->getWidth()) ?>" max-height="escapeHtmlAttr($block->getHeight()) ?>" From b0ccd4f9fed45147101195d71a7af298d4d8ff4b Mon Sep 17 00:00:00 2001 From: Alexander Aleman Date: Fri, 21 Feb 2020 12:13:24 +0100 Subject: [PATCH 3/6] correctly handle escaping --- .../Magento/Catalog/Block/Product/Image.php | 2 +- .../Catalog/Block/Product/ImageFactory.php | 21 ++----------------- .../frontend/templates/product/image.phtml | 6 +++++- .../product/image_with_borders.phtml | 6 +++++- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/app/code/Magento/Catalog/Block/Product/Image.php b/app/code/Magento/Catalog/Block/Product/Image.php index 7a7f9c0affc7d..ccc37029bedf7 100644 --- a/app/code/Magento/Catalog/Block/Product/Image.php +++ b/app/code/Magento/Catalog/Block/Product/Image.php @@ -14,7 +14,7 @@ * @method string getHeight() * @method string getLabel() * @method float getRatio() - * @method string getCustomAttributes() + * @method array getCustomAttributes() * @method string getClass() * @since 100.0.2 */ diff --git a/app/code/Magento/Catalog/Block/Product/ImageFactory.php b/app/code/Magento/Catalog/Block/Product/ImageFactory.php index 172cd794edfb9..2a0b31ec50942 100644 --- a/app/code/Magento/Catalog/Block/Product/ImageFactory.php +++ b/app/code/Magento/Catalog/Block/Product/ImageFactory.php @@ -67,23 +67,6 @@ public function __construct( $this->imageParamsBuilder = $imageParamsBuilder; } - /** - * Retrieve image custom attributes for HTML element - * - * @param array $attributes - * @return string - */ - private function getStringCustomAttributes(array $attributes): string - { - $result = []; - foreach ($attributes as $name => $value) { - if ($name != 'class') { - $result[] = $name . '="' . $value . '"'; - } - } - return !empty($result) ? implode(' ', $result) : ''; - } - /** * Retrieve image class for HTML element * @@ -161,7 +144,7 @@ public function create(Product $product, string $imageId, array $attributes = nu } $attributes = $attributes === null ? [] : $attributes; - + $data = [ 'data' => [ 'template' => 'Magento_Catalog::product/image_with_borders.phtml', @@ -170,7 +153,7 @@ public function create(Product $product, string $imageId, array $attributes = nu 'height' => $imageMiscParams['image_height'], 'label' => $this->getLabel($product, $imageMiscParams['image_type']), 'ratio' => $this->getRatio($imageMiscParams['image_width'], $imageMiscParams['image_height']), - 'custom_attributes' => $this->getStringCustomAttributes($attributes), + 'custom_attributes' => $attributes, 'class' => $this->getClass($attributes), 'product_id' => $product->getId() ], diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml index 2a251b00521d8..abd8038d266d9 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml @@ -7,7 +7,11 @@ getCustomAttributes() ?> + getCustomAttributes() as $name => $value): ?> + + escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" + + src="escapeUrl($block->getImageUrl()) ?>" width="escapeHtmlAttr($block->getWidth()) ?>" height="escapeHtmlAttr($block->getHeight()) ?>" diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml index 32d5ec8b288a9..0ea0ef1aa7a95 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml @@ -11,7 +11,11 @@ getCustomAttributes() ?> + getCustomAttributes() as $name => $value): ?> + + escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" + + src="escapeUrl($block->getImageUrl()) ?>" max-width="escapeHtmlAttr($block->getWidth()) ?>" max-height="escapeHtmlAttr($block->getHeight()) ?>" From 79025f879e94c8b409a7d792588e4fe30ea14f36 Mon Sep 17 00:00:00 2001 From: Alexander Aleman Date: Fri, 21 Feb 2020 13:18:08 +0100 Subject: [PATCH 4/6] Filter custom attributes and add changes to unit test --- .../Catalog/Block/Product/ImageFactory.php | 18 +++++++++++++++++- .../Unit/Block/Product/ImageFactoryTest.php | 7 +++++-- .../frontend/templates/product/image.phtml | 4 +--- .../templates/product/image_with_borders.phtml | 4 +--- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Catalog/Block/Product/ImageFactory.php b/app/code/Magento/Catalog/Block/Product/ImageFactory.php index 2a0b31ec50942..f025417095f29 100644 --- a/app/code/Magento/Catalog/Block/Product/ImageFactory.php +++ b/app/code/Magento/Catalog/Block/Product/ImageFactory.php @@ -67,6 +67,20 @@ public function __construct( $this->imageParamsBuilder = $imageParamsBuilder; } + /** + * Remove class from custom attributes + * + * @param array $attributes + * @return array + */ + private function filterCustomAttributes(array $attributes): array + { + if (isset($attributes['class'])) { + unset($attributes['class']); + } + return $attributes; + } + /** * Retrieve image class for HTML element * @@ -153,7 +167,7 @@ public function create(Product $product, string $imageId, array $attributes = nu 'height' => $imageMiscParams['image_height'], 'label' => $this->getLabel($product, $imageMiscParams['image_type']), 'ratio' => $this->getRatio($imageMiscParams['image_width'], $imageMiscParams['image_height']), - 'custom_attributes' => $attributes, + 'custom_attributes' => $this->filterCustomAttributes($attributes), 'class' => $this->getClass($attributes), 'product_id' => $product->getId() ], @@ -161,4 +175,6 @@ public function create(Product $product, string $imageId, array $attributes = nu return $this->objectManager->create(ImageBlock::class, $data); } + + } diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php index 95b06e40602bf..5a6fff4c729b2 100644 --- a/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php @@ -144,7 +144,7 @@ private function getTestDataWithoutAttributes(): array 'height' => 100, 'label' => 'test_image_label', 'ratio' => 1, - 'custom_attributes' => '', + 'custom_attributes' => [], 'product_id' => null, 'class' => 'product-image-photo' ], @@ -202,7 +202,10 @@ private function getTestDataWithAttributes(): array 'height' => 50, 'label' => 'test_product_name', 'ratio' => 0.5, // <== - 'custom_attributes' => 'name_1="value_1" name_2="value_2"', + 'custom_attributes' => [ + 'name_1' => 'value_1', + 'name_2' => 'value_2', + ], 'product_id' => null, 'class' => 'my-class' ], diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml index abd8038d266d9..fcda005c655f9 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image.phtml @@ -8,9 +8,7 @@ getCustomAttributes() as $name => $value): ?> - - escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" - + escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" src="escapeUrl($block->getImageUrl()) ?>" width="escapeHtmlAttr($block->getWidth()) ?>" diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml index 0ea0ef1aa7a95..6dcadfd2e4412 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml @@ -12,9 +12,7 @@ style="padding-bottom: getRatio() * 100) ?>%;"> getCustomAttributes() as $name => $value): ?> - - escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" - + escapeHtmlAttr($name) ?>="escapeHtmlAttr($value) ?>" src="escapeUrl($block->getImageUrl()) ?>" max-width="escapeHtmlAttr($block->getWidth()) ?>" From 5e84595da92ba3788edac27d8f35bc9a0e0e9d27 Mon Sep 17 00:00:00 2001 From: Alexander Aleman Date: Fri, 21 Feb 2020 13:19:59 +0100 Subject: [PATCH 5/6] Remove empty lines --- app/code/Magento/Catalog/Block/Product/ImageFactory.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/code/Magento/Catalog/Block/Product/ImageFactory.php b/app/code/Magento/Catalog/Block/Product/ImageFactory.php index f025417095f29..1dd55f75bdd0e 100644 --- a/app/code/Magento/Catalog/Block/Product/ImageFactory.php +++ b/app/code/Magento/Catalog/Block/Product/ImageFactory.php @@ -175,6 +175,4 @@ public function create(Product $product, string $imageId, array $attributes = nu return $this->objectManager->create(ImageBlock::class, $data); } - - } From afac7ead70c7c11329c56d3264ee51df655dc995 Mon Sep 17 00:00:00 2001 From: alexander-aleman <35915533+alexander-aleman@users.noreply.github.com> Date: Sat, 22 Feb 2020 10:36:36 +0100 Subject: [PATCH 6/6] Update expectation for return type of getCustomAttributes --- .../Magento/Swatches/Block/Product/ListProductTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Swatches/Block/Product/ListProductTest.php b/dev/tests/integration/testsuite/Magento/Swatches/Block/Product/ListProductTest.php index 460e4559a0e84..e6f566ac156db 100644 --- a/dev/tests/integration/testsuite/Magento/Swatches/Block/Product/ListProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Swatches/Block/Product/ListProductTest.php @@ -166,7 +166,7 @@ private function assertProductImage(array $images, string $area, array $expectat $this->updateProductImages($images); $productImage = $this->listingBlock->getImage($this->productRepository->get('configurable'), $area); $this->assertInstanceOf(Image::class, $productImage); - $this->assertEquals($productImage->getCustomAttributes(), ''); + $this->assertEquals($productImage->getCustomAttributes(), []); $this->assertEquals($productImage->getClass(), 'product-image-photo'); $this->assertEquals($productImage->getRatio(), 1.25); $this->assertEquals($productImage->getLabel(), $expectation['label']);