How an email marketing Magento extension can slow your PDPs to a crawl

We all know (right?) that when installing third-party extensions for Magento/Adobe Commerce, code review is essential. Bad practices can severely impact your site’s performance, its uptime, and even your security. Publication on the Magento marketplace does not necessarily guarantee that an extension will not have performance implications on your site. Nor should it mean that you consider it exempt from your own code review processes, whether that be through your agency or internal developers. All third-party extensions should be reviewed. Also, per PCI DSS requirement 6.2.3, you are required to independently review code when feasible.

A couple of years back I encountered an email marketing extension for a popular email and SMS platform, Attentive, that, if installed as is, can slow your PDPs to a crawl and has security issues to boot.

What did we find?

What we found was a smorgasbord of bad practices for Magento extensions.

1. Fetching way more attributes than are necessary to do the job.
2. N+1 queries.
3. Loading large amounts of data into memory.
4. Leaking product attribute values that you may not want to show on the storefront.
5. Leaking exception messages.

It all starts in their Helper/Helper.php file.

    /**
     * @throws NoSuchEntityException
     */
    public function getProductData(int $productId): ?array
    {
        try {
            $productRepoItem = $this->productRepository->getById($productId);
        } catch (NoSuchEntityException|Throwable $ex) {
            return null;
        }
        if ($productRepoItem->getExtensionAttributes() !== null) {
            if ($productRepoItem->getExtensionAttributes()->getConfigurableProductLinks() !== null) {
                $productRepoItemData['attentive_configurable_product_links']
                    = $productRepoItem->getExtensionAttributes()->getConfigurableProductLinks();
                $productVariantsData = [];
                $productVariants = $this->productCollectionFactory->create()
                    ->addAttributeToSelect('*')
                    ->addFieldToFilter('product_id', array_keys(
                        $productRepoItem->getExtensionAttributes()->getConfigurableProductLinks()));
                foreach ($productVariants as $productVariant) {
                    $productVariantsData[] = $productVariant->getData();
                }
                $productRepoItemData['attentive_variants'] = $productVariantsData;
            }
            if ($productRepoItem->getExtensionAttributes()->getConfigurableProductOptions() !== null) {
                $optionsData = array();
                foreach ($productRepoItem->getExtensionAttributes()->getConfigurableProductOptions() as $option) {
                    $optionsData[] = $option->getData();
                }
                $productRepoItemData['attentive_configurable_product_options'] = $optionsData;
            }
        }
        $typeInstance = $productRepoItem->getTypeInstance();
        if ($typeInstance instanceof Configurable && $productRepoItem instanceof Product) {
            $configurableAttributesData = array();
            $configurableAttributes = $typeInstance->getConfigurableAttributes($productRepoItem);
            foreach ($configurableAttributes as $attribute) {
                $configurableAttributesData[] = $attribute->getData();
            }
            $productRepoItemData['attentive_configurable_attributes'] = $configurableAttributesData;
            $configurableOptionsData = array();
            $configurableOptions = $typeInstance->getConfigurableOptions($productRepoItem);
            foreach ($configurableOptions as $configurableOption) {
                if ($configurableOption instanceof DataObject) {
                    $configurableOptionsData[] = $configurableOption->getData();
                } elseif (is_array($configurableOption)) {
                    $configurableOptionsData[] = $configurableOption;
                }
            }
            $productRepoItemData['attentive_configurable_options'] = $configurableOptionsData;
        }
        return $productRepoItemData;
    }

There are a few things going on here. If an item is a configurable product (in our use case that’s basically all of our PDPs, since we sell things by color and size), Attentive iterates all configurable children via the configurable options array, and then fetches ALL attribute data for that configurable product’s simple children, as well as for the configurable product itself.

This is problematic for a few of the aforementioned reasons.

Specifically:

1. A brand-new product collection with addAttributeToSelect('*') is created for every item in the customer’s cart (This method is called via the attentivemobile/magento2/Controller/Cart/Get.php in a loop). This is a query in a loop. For an item that has a color run of 10 with a size run of 10, that’s 100 expensive queries, since all attributes are fetched (each attribute fetched implies JOINs in Magento).
2. This loads ALL attributes for all children into memory, to then output. You could potentially run into out of memory errors on large configurable product pages.
3. Attentive most certainly does not use every attribute you use in your catalog.

Blast radius

Now - where is this used, and what is the blast radius of this?

It’s used on product pages via attentivemobile/magento2/view/frontend/templates/product_info.phtml.

This means that expensive operation runs on every one of your product pages.

Secondly, looking at what’s actually done with that data:

<?php

use Attentive\Integration\ViewModel\AttentiveProductInfoViewModel;
use Magento\Framework\View\Element\Template;

/* @var Template $block */

/**
 * @var AttentiveProductInfoViewModel $viewModel
 */
$viewModel = $block->getViewModel();
$escaper = $viewModel->getEscaper();
?>

<div id="attnProductInfo" style="display: none;" <?php

try {
    $str = $viewModel->getString();
    ?>
    data-product-info="<?=$escaper->escapeHtmlAttr($str)?>"
    <?php
} catch (Throwable $t) {
    try {
    ?>
    data-product-error="<?=$escaper->escapeHtmlAttr($t->getMessage())?>"
    <?php
    } catch (Throwable $t2) {
        ?>
        data-product-error="Cannot log error"
        <?php
    }
}
?>></div>

All of that attribute data is right there on your PDP. So if you have attributes that you might not want in the frontend… Congratulations, they’re now in the frontend. And unfortunately, Attentive isn’t the first extension I’ve seen that gobbles up every product attribute and/or potentially outputs things you might not want output. So they’re not alone in causing this particular type of security issue.

Third, take a look at that data-product-error line. That leaks exceptions to the frontend, which could include sensitive data, regardless of your settings for errors and exceptions in Magento. That’s potentially the worse security issue of the two.

FAQ

Shouldn’t you report these issues?

Yes. Already did when I discovered them. Still not fixed. So it would be better if the community were aware of this stuff. Maybe writing up a detailed analysis will cause them to fix it. Or maybe it will get fewer people to make the same mistakes. Like I mentioned, these issues are hardly unique (although the exception leaking issue certainly is).

Shouldn’t you give them a pull request?

There isn’t a public GitHub repository. However, you can download their extension for free from the Adobe Marketplace to get a copy and verify my claims. If there isn’t a public repository, I can’t really fork it and fix it. I also, architecturally, disagree with their approach. So for my purposes, it would be a waste of my time to fix something that is fundamentally flawed in the first place.

What do we do about this?

Well - you could patch getProductData() to just return null so it doesn’t run at all. That’s kind of a blunt way of “fixing” it.

However, my actual recommendation is building your own catalog integration using the Attentive catalog API (the documentation can be found here) and sending them the catalog asynchronously, rather than letting their tag pick up catalog items on the fly as customers visit PDPs. Sorry, I can’t share that code with you. Their approach is problematic on its own terms, because it only results in them getting catalog data from you when a customer visits a PDP. So for low traffic pages, and for your newest arrivals, those products aren’t in their catalog until someone has visited the page.