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 5a31f3d125c8..ef2ba856fad8 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 33f7620f1a1f..8d0cd0294c48 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 ef2ba856fad8..2a251b00521d 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 8d0cd0294c48..32d5ec8b288a 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 7a7f9c0affc7..ccc37029bedf 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 172cd794edfb..2a0b31ec5094 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 2a251b00521d..abd8038d266d 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 32d5ec8b288a..0ea0ef1aa7a9 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 2a0b31ec5094..f025417095f2 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 95b06e40602b..5a6fff4c729b 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 abd8038d266d..fcda005c655f 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 0ea0ef1aa7a9..6dcadfd2e441 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 f025417095f2..1dd55f75bdd0 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 460e4559a0e8..e6f566ac156d 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']);