Fix TypeError in Shipping::collectRates() when no carriers are configured - #41169
Open
lbajsarowicz wants to merge 1 commit into
Open
Fix TypeError in Shipping::collectRates() when no carriers are configured#41169lbajsarowicz wants to merge 1 commit into
lbajsarowicz wants to merge 1 commit into
Conversation
getValue('carriers', ...) returns null instead of an empty array when no
carrier module contributes config to the scope (e.g. all shipping carriers
disabled), causing foreach() to throw on PHP 8. Normalize to an array
before iterating, matching the guard already used for $limitCarrier a few
lines below.
Contributor
Author
|
@magento run all tests |
|
Hi @lbajsarowicz. 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. |
Contributor
Author
|
@magento run Unit Tests |
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
Magento\Shipping\Model\Shipping::collectRates()iterates the carrier configuration without checking it is an array:Magento\Config\App\Config\Type\System::getDataByPathParts()returnsnull, not an empty array, when the requested key is absent from the merged scope data. That happens whenever no carrier module contributes acarriers/*node for the scope — for example with every shipping carrier disabled or uninstalled. On PHP 8 theforeachthen throws aTypeErrorand checkout breaks.ScopeConfigInterface::getValue()is declared@return mixed, so callers cannot assume an array.Note the asymmetry a few lines below in the same method: the
$limitCarrierbranch already normalises its value withis_array()before iterating. This applies the same guard to the branch that lacks it.This is the same bug that was half-fixed in 2021
#30830 reported exactly this failure ("disable all shipping carrier modules" → blank checkout,
array_keys(): argument #1 must be of type array, null given). It was fixed by #30822, which guarded the identicalgetValue('carriers', ...)call sites inMagento\Shipping\Model\Config::getActiveCarriers()andgetAllCarriers()— but did not touchShipping::collectRates(), which has the same unguarded pattern and is still reachable today.Fixed Issues
No open issue; this is the remaining half of the defect reported in #30830, which was closed when #30822 fixed the sibling call sites.
Manual testing scenarios
Stores > Configuration > Sales > Delivery Methods, or uninstall the carrier modules).Shipping::collectRates()with a request that has nolimitCarrier.TypeError: foreach() argument must be of type array|object, null given. After: rate collection completes and returns no rates.Questions or comments
Gates run locally on
2.4-develop(Warden, PHP 8.3):Shipping/Test/Unit/Model/ShippingTest.php— 3 tests pass, including a new case that mocks the config to returnnulland assertscollectRates()returns normally instead of throwing.Magento2: clean. PHPStan level 1: no errors.Shipping.phpand passes with the fix.No signature changes, so no Semantic Version Checker exposure.
Contribution checklist