Guard identifier key access and declare callerPage property in ApplePay#1239
Open
mmaymo wants to merge 1 commit into
Conversation
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.
What and why
During the Apple Pay checkout flow, two PHP notices were being written to the error log on every request: a
PHP Warningfor an undefined array keyidentifierinResponsesToApple::reorderShippingMethods, and aPHP Deprecatednotice for dynamic property creation of$callerPageonApplePayDataObjectHttp. Neither noticeblocked checkout or affected the user experience, but both polluted error logs and represent
forward-compatibility risks — dynamic property creation is deprecated in PHP 8.2 and will become a fatal error
in a future PHP version, while undefined array key access is already an
E_WARNINGthat strict errorconfigurations can surface as an exception.
How it works now
In
ResponsesToApple::reorderShippingMethods, the direct comparison$method['identifier'] === $selectedShippingMethod['identifier']has been replaced with($method['identifier'] ?? null) === ($selectedShippingMethod['identifier'] ?? null), so a missingidentifierkey on either side returnsnullinstead of raising a warning. In
ApplePayDataObjectHttp,$callerPageis now declared as an explicitprotectedclass property with a@var string|nulldocblock, which meansassignDataObjectValuesno longercreates it as a dynamic property when it assigns values from POST data.
What to verify in review
Covered by unit tests
identifierkey access inResponsesToApple::reorderShippingMethodsis guarded with??so a missingkey does not raise a PHP Warning
$callerPageproperty is explicitly declared in theApplePayDataObjectHttpclass body and is nolonger assigned dynamically
Not covered by unit tests
PHP Deprecatednotice for$callerPageis logged — structurally guaranteed once the property iscreates it as a dynamic property when it assigns values from POST data.