ENGCOM-6962: Correctly escape custom product image attributes #26959

magento · Mar 15, 2020 · 16247f6 · 16247f6
2 parents 9a9e9c9 + 7a7adcf
commit 16247f6
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 16 deletions.
diff --git 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
@@ -68,20 +68,17 @@ public function __construct(
     }
 
     /**
-     * Retrieve image custom attributes for HTML element
+     * Remove class from custom attributes
      *
      * @param array $attributes
-     * @return string
+     * @return array
      */
-    private function getStringCustomAttributes(array $attributes): string
+    private function filterCustomAttributes(array $attributes): array
     {
-        $result = [];
-        foreach ($attributes as $name => $value) {
-            if ($name != 'class') {
-                $result[] = $name . '="' . $value . '"';
-            }
+        if (isset($attributes['class'])) {
+            unset($attributes['class']);
         }
-        return !empty($result) ? implode(' ', $result) : '';
+        return $attributes;
     }
 
     /**
@@ -170,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'] ?? 0, $imageMiscParams['image_height'] ?? 0),
-                'custom_attributes' => $this->getStringCustomAttributes($attributes),
+                'custom_attributes' => $this->filterCustomAttributes($attributes),
                 'class' => $this->getClass($attributes),
                 'product_id' => $product->getId()
             ],

diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/ImageFactoryTest.php
@@ -145,7 +145,7 @@ private function getTestDataWithoutAttributes(): array
                     'height' => 100,
                     'label' => 'test_image_label',
                     'ratio' => 1,
-                    'custom_attributes' => '',
+                    'custom_attributes' => [],
                     'product_id' => null,
                     'class' => 'product-image-photo'
                 ],
@@ -203,7 +203,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
@@ -10,7 +10,9 @@
 ?>
 
 <img class="photo image <?= $escaper->escapeHtmlAttr($block->getClass()) ?>"
-    <?= $escaper->escapeHtml($block->getCustomAttributes()) ?>
+    <?php foreach ($block->getCustomAttributes() as $name => $value): ?>
+        <?= $escaper->escapeHtmlAttr($name) ?>="<?= $escaper->escapeHtmlAttr($value) ?>"
+    <?php endforeach; ?>
     src="<?= $escaper->escapeUrl($block->getImageUrl()) ?>"
     loading="lazy"
     width="<?= $escaper->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
@@ -14,7 +14,9 @@
     <span class="product-image-wrapper"
           style="padding-bottom: <?= ($block->getRatio() * 100) ?>%;">
         <img class="<?= $escaper->escapeHtmlAttr($block->getClass()) ?>"
-            <?= $escaper->escapeHtmlAttr($block->getCustomAttributes()) ?>
+            <?php foreach ($block->getCustomAttributes() as $name => $value): ?>
+                <?= $escaper->escapeHtmlAttr($name) ?>="<?= $escaper->escapeHtmlAttr($value) ?>"
+            <?php endforeach; ?>
             src="<?= $escaper->escapeUrl($block->getImageUrl()) ?>"
             loading="lazy"
             width="<?= $escaper->escapeHtmlAttr($block->getWidth()) ?>"

diff --git 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']);