Dangerous bug with virtual category module

[Nuranto]

Preconditions

Magento Version : 2.4.4

ElasticSuite Version :2.10.10

Environment : PHP 8.1

Steps to reproduce

1. Enable virtual categories
2. Load a category page on frontend, for examples : domain.com/iphones
3. try to reach : domain.com/something/iphones, domain.com/any/thing/really/iphones

Expected result

1. 404 on both URLs

Actual result

1. Category iphones gets displayed for both URLs

Which can be dangerous for SEO. In our case we do have some wrong URLs indexed already (we don't know yet how many and why they get indexed (probably wrong relative links in some cms page)).

If I disable virtualCategory module, problem is solved. I was able to reproduce the error on multiple projects.

[Nuranto]

I don't understand the logic of Smile\ElasticsuiteVirtualCategory\Controller\Router : Why are we rerouting non-virtual categories (and products) here ?

But if there is a reason, then I can see that getCategoryRewrite is wrong. We should use the whole path (var $identifier) to determine the category route instead of last chunk because, as it is now :

1. We can have wrong URLs like domain.com/any/thing/really/iphones that gives a result when it should not.
2. I bet there is another bug for categories using same url_key like domain.com/bags/blue & domain.com/tshirts/blue

[RyseSlade]

I also have a problem with the latest changes in the Smile\ElasticsuiteVirtualCategory\Model\Url class which added the method getCategoryRewrite. When the shop is configured to use ".html" suffix for categories the method will load the category and redirect if a rewrite exists. This means now categories can also be called like this https://www.example.com/category but it should only work with https://www.example.com/category.html. Just another SEO issue.

For now I will create a plugin afterGetCategoryRewrite to fix this...

[romainruaud]

@RyseSlade instead of creating a plugin in your own project to fix this, can you propose a PR for us here ?

I'll gladly merge it :)

Regards

[RyseSlade]

Well, I did not "fix" the issue. I just removed the category rewrite logic with this plugin as I don't need/want that rewrite to happen.

public function afterGetCategoryRewrite(Url $subject, $result, $categoryPath, $storeId)
{
    return null;
}

[romainruaud]

I don't understand the logic of Smile\ElasticsuiteVirtualCategory\Controller\Router : Why are we rerouting non-virtual categories (and products) here ?

It's due to https://github.com/Smile-SA/elasticsuite/issues/49 and the implementation on https://github.com/Smile-SA/elasticsuite/pull/2489

The usecase here is easy to understand, you can create a Virtual Category "special sales" that will contains "all discounted products".

If you check the "Generate Virtual Category Root Subtree", you will see a category filter on the "special sales" category.

Eg you will be on "Special Sales" category and see these values for categories filter :

• Men
• Women

If you choose to filter on "Men", you'll go to the www.myshop.com/special-sales/men.html page.

This page is not intended to exist, since "special sales" category has no child categories named "Men".

That's why we made this router.

I agree that it's still perfectible, especially regarding the fact that we don't check if "special-sales" in the url is something that is corresponding with an existing category.

We'll try to improve this.

regards

[Nuranto]

Thanks for the explanation @romainruaud, I now understand the context !

Looking forward for the fix !

[igorwulff]

We've decided to just remove the following logic with a patch from the Virtual Category router (vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php):

        $categoryRewrite = $this->getCategoryRewrite($identifier);
        if ($categoryRewrite) {
            $request->setAlias(UrlInterface::REWRITE_REQUEST_PATH_ALIAS, $identifier);
            $request->setPathInfo('/' . $categoryRewrite->getTargetPath());

            return $this->actionFactory->create('Magento\Framework\App\Action\Forward');
        }

This way we will get an error during a composer install whenever changes have been made in this logic, since this is inherently a bug which I believe we should not fix with plugins or observers as done by @RyseSlade.

[romainruaud]

The PR #2743 contains a fix for this one.

I just added a guard fonction early on the Router, to check if we are "under the generated subtree of a virtual category" (see my previous comment for explanations). If that's not the case, we just return null and let the other Magento routers try to handle the request.

Let me know if that's okay for you, this thing should make it fast to our 2.10.12 version.

Regards

[misthero]

The PR https://github.com/Smile-SA/elasticsuite/pull/2743 contains a fix for this one.

this cause all products under virtual categories to return 404

[romainruaud]

@misthero are you using the "Use Categories Path for Product URLs" option ?

[misthero]

@misthero are you using the "Use Categories Path for Product URLs" option ?

yes, the config is this:

reverting to version 2.10.11 from 2.10.12 fixes the problem

also "Generate Virtual Category Root Subtree" is unchecked.

[romainruaud]

@misthero can you check that the fix in this PR solves your issue ? https://github.com/Smile-SA/elasticsuite/pull/2748

If yes, I'll tag a 2.10.12.1 quickly.

Regards

[nghiadxvn]

@romainruaud as I checked, the issue came from this file smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/VirtualCategory/Root.php ( the getByUrlKeys function). If you didn't enable "Generate Virtual Category Root Subtree" for category, the product will direct to 404 page.

[romainruaud]

@nghiadxvn can you check my PR #2748 ?

[nghiadxvn]

@romainruaud yes. It works

[nghiadxvn]

Hi @romainruaud, do you have a schedule for releasing this fix?

[misthero]

@misthero can you check that the fix in this PR solves your issue ? #2748

If yes, I'll tag a 2.10.12.1 quickly.

Regards

It works!

[romainruaud]

2.10.12.1 has been tagged :)

You can update to this version.

I close the issue.