Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid undefined indexes in notifications #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Avoid undefined indexes in notifications #20

wants to merge 3 commits into from

Conversation

twisted1919
Copy link

If you have a webhook waiting information, with code similar to :

/* parse the incoming request in a meaningful response */
$response = app()->paymentManager->getProvider('2checkout')->getGateway()->acceptNotification()->send();

And someone does a post request without any data, it will trigger warnings like Undefined index: sale_id because there is basically no check made to see if the given array keys do exist.

This pull request checks using isset to make sure the array keys do exist before trying to fetch their value. Initially i thought to set the values to empty strings if they are not set, but in my second commit i used null as it makes more sense in this context. I also included a test for it.

Please keep in mind i am not familiar at all with how your library works internally, so please review the changes carefully.

Thank you.

@coveralls
Copy link

coveralls commented Jul 5, 2018

Coverage Status

Coverage increased (+0.4%) to 64.858% when pulling b92c0c4 on twisted1919:master into c317aad on collizo4sky:master.

@xwiz
Copy link

xwiz commented Dec 7, 2022

Please merge @w3guy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants