braintree / braintree_php

Braintree PHP library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
547 stars 224 forks source link

Fix bug where method_exists() may be called on a null value #309

Closed robbieaverill closed 2 years ago

robbieaverill commented 2 years ago

Summary

Braintree SDK: 6.5.0 PHP: 8.0.5

When ->toArray() is called on Braintree responses, it sometimes throws a type error when treating null as an object:

method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given

Checklist

robbieaverill commented 2 years ago

Reviewers: I'll update the changelog conflict once this has been reviewed.

strix commented 2 years ago

The very useful toArray method is broken on php 8.x without this fix in place. I'm sure it's already known but I'll say it anyway:

On php 7.x, calling method_exists on null returns false. In php 8.x, calling method_exists on null throws a fatal error.

You can test this on any php online | sandbox, by selecting php 7.x and then php 8.x with the following code:

class Test {
    public function blah(){
        return 'blah';
    }
}

$test = new Test();
$testNull = null;

$methodExistsObject = method_exists($test, 'blah');
$methodExistsNull = method_exists($testNull, 'blah');

var_dump($methodExistsObject);
var_dump($methodExistsNull);
/*
PHP 7.x Output:
bool(true)
bool(false)

PHP 8.x Output:
bool(true)
Fatal error: Uncaught TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given
*/

This PR fixes this issue by guarding the call to method_exists with a is_object check on the value. The result ends up being the same between PHP 7.x and PHP 8.x and I'm using this solution currently to make the toArray method work on PHP 8.1

Looking forward to this getting merged in.

robbieaverill commented 2 years ago

cc @jplukarski

WalkerKarina commented 2 years ago

Hi, thanks for leaving a PR @robbieaverill. Small thing could you please mark the changelog as unreleased? Otherwise it looks good to me thanks!

robbieaverill commented 2 years ago

@WalkerKarina thanks, I've updated the changelog

Epreuve commented 2 years ago

Hey @robbieaverill Sorry about this getting bumped around for so long, considering it was a pretty quick fix. We included this and another similar fix in the 6.9.0 release.

If you come across any other issues let us know via an issue or PR, and we will try to be a bit quicker getting those fixed up or merged going forward