Check correct variable for null #3398
Conversation
m2-community-project
bot
added
Progress: ready for review
and removed
Progress: ready for review
labels
The wrong variable is being checked for null, which can cause a null reference exception
dennisvanderweide
force-pushed
the
patch-1
branch
from
df25e96 to
d892fff
Compare
|
@magento run all tests |
|
|
||
| if ($orgSourceItem !== null) { | ||
| $status = (int) $orgSourceItem[SourceItemInterface::STATUS]; | ||
| } elseif ($dstSourceItemQty !== null) { |
There was a problem hiding this comment.
Hello @dennisvanderweide,
Thank you for your contribution!
Please let us know in which scenario the existing will create an issue.
Also please help us with the manual testing scenario.
Thank you
There was a problem hiding this comment.
@engcom-Hotel In case $dstSourceItem is null, $dstSourceItemQty will be 0.0 (line 124). If $orgSourceItem is also null it will trigger this if statement. $dstSourceItemQty !== null will be false, because it will be 0.0, but dstSourceItem is still null. $dstSourceItem[SourceItemInterface::STATUS] then gives an error.
I don't know how $orgSourceItem and $dstSourceItem can be both null, both this happend with one of our customers and I saw that this if statement doesn't handle this correctly.
There was a problem hiding this comment.
Make sense to me thank you for the clarification.
engcom-Hotel
moved this from Ready for Review
to Changes Requested
in Inventory - Pull Request Progress
engcom-Hotel
added
Progress: needs update
and removed
Progress: ready for review
labels
There was a problem hiding this comment.
Approving this PR.
@dennisvanderweide requesting you to provide some manual testing scenario to test the functionality.
engcom-Hotel
added
Progress: ready for testing
and removed
Progress: needs update
labels
engcom-Hotel
moved this from Changes Requested
to Reviewer Approved
in Inventory - Pull Request Progress
|
@engcom-Hotel Well, it's almost a year ago that I created this PR, so I don't really remember what our customer did to have this issue. |
|
✔️ QA Passed No manual testing scenario required. The PR's purpose is to correct a logical error in variable checking that causes the system to crash when attempting to read the status property from a null source item object. It's a simple but critical fix that ensures proper null validation before array access, preventing fatal errors during bulk inventory transfer operations. Hence moving it further for merging. |
|
@magento run all tests |
engcom-Hotel
moved this from Reviewer Approved
to Ready for Delivery
in Inventory - Pull Request Progress
2 participants
Description (*)
The wrong variable is being checked for null, which can cause a null reference exception in the bulk source transfer
Contribution checklist (*)