Fix TypeError when the type param is missing on the admin new product page - #41105
Open
michielgerritsen wants to merge 1 commit into
Open
Fix TypeError when the type param is missing on the admin new product page#41105michielgerritsen wants to merge 1 commit into
michielgerritsen wants to merge 1 commit into
Conversation
Magento\Catalog\Controller\Adminhtml\Product\NewAction::execute() passes the raw 'type' request param straight into Magento\Framework\RegexValidator::validateParamRegex(), which is typed as string. Opening admin/catalog/product/new without a type param makes getParam() return null and the request dies with a TypeError instead of forwarding to noroute. An array param (?type[]=simple) fails the same way. Guard the call like Sales\Controller\Adminhtml\Order\Create\LoadBlock already does, and forward to noroute for any non-string type.
|
Hi @michielgerritsen. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
4 tasks
Member
Author
|
@magento run all tests |
Contributor
|
@magento create issue |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (*)
Opening the admin "New Product" page without a
typerequest parameter throws a fatalTypeErrorinstead of forwarding tonoroute:NewAction::execute()passes the raw request param straight intoRegexValidator::validateParamRegex(), which is declared asstring $params.RequestInterface::getParam()returnsnullwhen the parameter is absent, so the request dies with a 500. The same happens for an array param (?type[]=simple), which producesarray given.Magento\Sales\Controller\Adminhtml\Order\Create\LoadBlockalready guards its call to the same method;NewActiondoes not. This change applies the same guard and forwards tonoroutefor any non-stringtype, so the validation introduced in AC-7025 keeps working while an absent parameter no longer 500s.Related Pull Requests
The same fix submitted to Mage-OS: mage-os/mageos-magento2#318
Fixed Issues (if relevant)
No linked GitHub issue; reported from a production
exception.log.Manual testing scenarios (*)
admin/catalog/product/new/(notypeand nosetin the URL).TypeErrorinvar/log/exception.log.noroutepage.admin/catalog/product/new/?type[]=simple.TypeError ... array given.noroutepage.admin/catalog/product/new/set/4/type/simple/URL and confirm the new product form still loads.typeparam containing layout-handle injection characters still forwards tonoroute.Questions or comments
The alternative fix would be widening
RegexValidator::validateParamRegex()to accept?string. That changes a public method signature on a non-final framework class, so I kept the change in the caller instead, matching the existingLoadBlockpattern. Happy to switch approaches if you prefer the framework-level fix.Contribution checklist (*)
Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\NewActionTest::testExecuteWithoutTypeParamand::testExecuteWithNonStringTypeParam, both reproduce the reportedTypeErrorwithout the fix)vendor/bin/phpunit -c dev/tests/unit/phpunit.xml.dist app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/NewActionTest.php)Resolved issues: