Missing return value in Magento\CmsUrlRewrite\Plugin\Cms\Model\Store\View::afterSave can cause saving a storeview to crash

[hostep]

Preconditions (*)

1. Tested on a vanilla Magento 2.3.5 and 2.4-develop (commit 735579d) instance

Steps to reproduce (*)

1. Setup a vanilla Magento installation
2. In the backend, go to Stores > All Stores
3. Open the 'Default Store View'
4. Save the storeview, no need to change anything, notice that this works fine
5. Now simulate we installed a bunch of custom modules which changes the sort order of certain Magento modules in the app/etc/config.php file, so manually change that file so Magento_CatalogUrlRewrite comes later then Magento_CmsUrlRewrite:

--- app/etc/config.php	2020-07-08 11:42:38.000000000 +0200
+++ app/etc/config2.php	2020-07-08 11:42:35.000000000 +0200
@@ -30,7 +30,6 @@
         'Magento_Payment' => 1,
         'Magento_CatalogRuleGraphQl' => 1,
         'Magento_CatalogRule' => 1,
-        'Magento_CatalogUrlRewrite' => 1,
         'Magento_StoreGraphQl' => 1,
         'Magento_Widget' => 1,
         'Magento_Quote' => 1,
@@ -39,6 +38,7 @@
         'Magento_CmsGraphQl' => 1,
         'Magento_EavGraphQl' => 1,
         'Magento_CmsUrlRewrite' => 1,
+        'Magento_CatalogUrlRewrite' => 1,
         'Magento_CmsUrlRewriteGraphQl' => 1,
         'Magento_User' => 1,
         'Magento_Msrp' => 1,

6. Flush the caches: bin/magento cache:flush (do not run bin/magento setup:upgrade because it will revert the changes you just did)
7. In the backend, go to Stores > All Stores and open the 'Default Store View' again
8. Save the storeview again and notice it throws the following error:

TypeError: Argument 2 passed to Magento\CatalogUrlRewrite\Model\Category\Plugin\Store\View::afterSave() must be an instance of Magento\Store\Model\ResourceModel\Store, null given, called in lib/internal/Magento/Framework/Interception/Interceptor.php on line 146 and defined in app/code/Magento/CatalogUrlRewrite/Model/Category/Plugin/Store/View.php:103 Stack trace:
#0 lib/internal/Magento/Framework/Interception/Interceptor.php(146): Magento\CatalogUrlRewrite\Model\Category\Plugin\Store\View->afterSave(Object(Magento\Store\Model\ResourceModel\Store\Interceptor), NULL, Object(Magento\Store\Model\Store\Interceptor))
#1 lib/internal/Magento/Framework/Interception/Interceptor.php(153): Magento\Store\Model\ResourceModel\Store\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Store\Model\Store\Interceptor))
#2 generated/code/Magento/Store/Model/ResourceModel/Store/Interceptor.php(86): Magento\Store\Model\ResourceModel\Store\Interceptor->___callPlugins('save', Array, Array)
#3 lib/internal/Magento/Framework/Model/AbstractModel.php(654): Magento\Store\Model\ResourceModel\Store\Interceptor->save(Object(Magento\Store\Model\Store\Interceptor))
#4 lib/internal/Magento/Framework/Interception/Interceptor.php(58): Magento\Framework\Model\AbstractModel->save()
#5 lib/internal/Magento/Framework/Interception/Interceptor.php(138): Magento\Store\Model\Store\Interceptor->___callParent('save', Array)
...

Expected result (*)

1. Storeview gets saved without errors
2. All after plugins should return a value, in this case Magento\CmsUrlRewrite\Plugin\Cms\Model\Store\View::aftersSave is missing one

Actual result (*)

1. Saving a storeview results in an error
2. The after plugin is missing a return value

Discussion

Error was introduced in Magento 2.3.4 by MC-18561

Following patch seems to resolve the issue:

--- vendor/magento/module-cms-url-rewrite/Plugin/Cms/Model/Store/View.php   2020-01-10 06:20:40.000000000 +0100
+++ /vendor/magento/module-cms-url-rewrite/Plugin/Cms/Model/Store/View.php   2020-07-08 11:25:17.000000000 +0200
@@ -67,16 +67,18 @@
      * @param ResourceStore $object
      * @param ResourceStore $result
      * @param ResourceStore $store
-     * @return void
+     * @return ResourceStore
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function afterSave(ResourceStore $object, ResourceStore $result, AbstractModel $store): void
+    public function afterSave(ResourceStore $object, ResourceStore $result, AbstractModel $store): ResourceStore
     {
         if ($store->isObjectNew() || $store->dataHasChangedFor('group_id')) {
             $this->urlPersist->replace(
                 $this->generateCmsPagesUrls((int)$store->getId())
             );
         }
+
+        return $result;
     }

     /**

---

Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.

• Severity: S0 - Affects critical data or functionality and leaves users without workaround.
• Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
• Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
• Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
• Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

(Not sure if S0 is correct here, creating a storeview feels like critical functionality, but I'm fine with S2 as well ...)

[m2-assistant]

Hi @hostep. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[magento-engcom-team]

✅ Confirmed by @engcom-Alfa
Thank you for verifying the issue. Based on the provided information internal tickets MC-36312 were created

Issue Available: @engcom-Alfa, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

[slavvka]

Hi @hostep. Thank you for your report.
The issue has been fixed in #29035 by @hostep in 2.4-develop branch
Related commit(s):

• 1a73d0ef24ac4fabfeb493f34414bdce20355f7f
• 87ce9e786b64dc737466197b640034b2e7de003f
• 39481a5bda95e36da22f9636335c178e46c781c5

The fix will be available with the upcoming 2.4.1 release.

[hostep]

Thanks @slavvka! Can you confirm that this bugfix will be backported to 2.3.x? Since the bug was introduced in 2.3.4. Thanks!

[mad-develop]

@slavvka is a backport coming? Issue is still present in 2.3.7

[hostep]

@mad-develop: nope, Magento gave up on fixing "small bugs" for the 2.3.x release line when 2.4.0 became available if I remember correctly

Also, all following 2.3.7+ releases will only be security fixes (and maybe super high prio bug fixes?) as far as I know.

But to fix this problem on your 2.3.x installation, you can take this patch and try to apply it via https://github.com/cweagans/composer-patches or https://github.com/vaimo/composer-patches/ or some other way, that's how we deal with these things 🙂