Validating non existing customergroups creates error

i3rinkmann commented 1 year ago

Description

The problem is with the URL /registerFC/index/sValidation/ and calling it with an invalid customerGroupKey. In the default theme there is an somewhat readable error message but only because the exception is raised in the register.php. We have a custom theme in one of our shops and there only the stacktrace is shown which is not really helpful.

In my opinion calling the URL with an wrong customerGroupKey should not throw an exception but instead just inform you that the Group does not exists.

PHP Version

7.4.33

Shopware Version

5.7.18

How to reproduce

call the URL http://your.shop.com/registerFC/index/sValidation/

Where the NonExistingCustomerGroupKey is a customerGroupKey that does not exist

Expected behaviour

The site catches the error and gives a human readable error message such as "Customer Group with the Key does not exist.

Actual behaviour

Ups! Ein Fehler ist aufgetreten! Die nachfolgenden Hinweise sollten Ihnen weiterhelfen.

Invalid customergroup in /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Controllers/Frontend/Register.php on line 467

Stack trace:

#0 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Controllers/Frontend/Register.php(492): Shopware_Controllers_Frontend_Register->getCustomerGroupKey()
#1 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Controllers/Frontend/Register.php(87): Shopware_Controllers_Frontend_Register->getRegisterData()
#2 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Library/Enlight/Controller/Action.php(188): Shopware_Controllers_Frontend_Register->indexAction()
#3 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Library/Enlight/Controller/Dispatcher/Default.php(467): Enlight_Controller_Action->dispatch('indexAction')
#4 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Library/Enlight/Controller/Front.php(226): Enlight_Controller_Dispatcher_Default->dispatch(Object(Enlight_Controller_Request_RequestHttp), Object(Enlight_Controller_Response_ResponseHttp))
#5 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Kernel.php(195): Enlight_Controller_Front->dispatch()
#6 /www/htdocs/w019b7ce/shopware.wantedweb.de/vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php(85): Shopware\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#7 /www/htdocs/w019b7ce/shopware.wantedweb.de/vendor/symfony/http-kernel/HttpCache/HttpCache.php(479): Symfony\Component\HttpKernel\HttpCache\SubRequestHandler::handle(Object(Shopware\Kernel), Object(Symfony\Component\HttpFoundation\Request), 1, true)
#8 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Components/HttpCache/AppCache.php(270): Symfony\Component\HttpKernel\HttpCache\HttpCache->forward(Object(Symfony\Component\HttpFoundation\Request), true, NULL)
#9 /www/htdocs/w019b7ce/shopware.wantedweb.de/vendor/symfony/http-kernel/HttpCache/HttpCache.php(452): Shopware\Components\HttpCache\AppCache->forward(Object(Symfony\Component\HttpFoundation\Request), true)
#10 /www/htdocs/w019b7ce/shopware.wantedweb.de/vendor/symfony/http-kernel/HttpCache/HttpCache.php(346): Symfony\Component\HttpKernel\HttpCache\HttpCache->fetch(Object(Symfony\Component\HttpFoundation\Request), true)
#11 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Components/HttpCache/AppCache.php(196): Symfony\Component\HttpKernel\HttpCache\HttpCache->lookup(Object(Symfony\Component\HttpFoundation\Request), true)
#12 /www/htdocs/w019b7ce/shopware.wantedweb.de/vendor/symfony/http-kernel/HttpCache/HttpCache.php(224): Shopware\Components\HttpCache\AppCache->lookup(Object(Symfony\Component\HttpFoundation\Request), true)
#13 /www/htdocs/w019b7ce/shopware.wantedweb.de/engine/Shopware/Components/HttpCache/AppCache.php(117): Symfony\Component\HttpKernel\HttpCache\HttpCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#14 /www/htdocs/w019b7ce/shopware.wantedweb.de/shopware.php(122): Shopware\Components\HttpCache\AppCache->handle(Object(Symfony\Component\HttpFoundation\Request))
#15 {main}

aragon999 commented 1 year ago

I looked into it, so one can trigger the error using for example the following URL: http://your.shop.com/registerFC/index/sValidation/G

I would solve it, that in the case if an invalid customer group is used we simply use the fallback customer group. I would not even display an error message, as such a link should never exist. What do you think?

Furthermore, you probably can replace registerFC by register in the routes. It currently is an alias: https://github.com/shopware5/shopware/blob/f56e358f200efaaf0e1f0227c0127c69b8da7195/engine/Shopware/Plugins/Default/Core/ViewportForward/Bootstrap.php#L86-L91

Which might get removed somewhere in the future :-)

i3rinkmann commented 1 year ago

@aragon999 In our case it would actually be helpful to have some kind of error. In the current shop there is a plugin that reroutes customers that are business customers to the validation form. If it simply gets rerouted again to the fallback (i guess EK) then we would not know if that happens and some admin would have to check if they are business customers or not.

The error actually showed us that the customer group "Händler" got deleted after the update which is a problem in itself because the group was not deleted in the backend (there was no log message indicating its removal) and i was not able to determine how it got deleted.

With the proposed fallback this would have been way harder to find.

mitelg commented 1 year ago

I would like to second what @i3rinkmann said, as I wrote already in the PR a few minutes ago :blush: :+1:

shopware5 / shopware