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

Array to string conversion in CIMAbstractResponse #105

Closed
javabudd opened this issue Jan 30, 2018 · 7 comments
Closed

Array to string conversion in CIMAbstractResponse #105

javabudd opened this issue Jan 30, 2018 · 7 comments

Comments

@javabudd
Copy link

The function getResultCode in the CIMAbstractResponse class (line 59) is doing an array to string conversion which now throws a fatal error in PHP 7.2.

$result should be evaluated before being casted to a string

@judgej
Copy link
Member

judgej commented Jan 31, 2018

https://github.com/thephpleague/omnipay-authorizenet/blob/master/src/Message/CIMAbstractResponse.php#L61

Is this about the changes to operator precedence? This is the line now:

$result = (string)$this->data['messages'][0]['resultCode'];

TBH I'm not sure why the value is cast to a string, since you can't cast an array to a string anyway. If you take out the (string) completely, does it work for you? I've not got PHP7.2 handy to try it on, though we could enable on travis.

Edit: Just noticed I have got 7.2 available to install. Just doing that now.

@judgej
Copy link
Member

judgej commented Jan 31, 2018

Not sure where the problem is exactly, but it seems to be the way the XML is parsed. This is what the response looks like in 7.1:

array(2) {
  ["messages"]=>
  array(1) {
    [0]=>
    array(2) {
      ["resultCode"]=>
      string(2) "Ok"
      ["message"]=>
      array(1) {
        [0]=>
        array(2) {
          ["code"]=>
          string(6) "I00001"
          ["text"]=>
          string(11) "Successful."
        }
      }
    }
  }

Then in 7.2 the response looks like this:

array(2) {
  ["messages"]=>
  array(1) {
    [0]=>
    array(2) {
      ["resultCode"]=>
      array(1) {
        [0]=>
        array(0) {
        }
      }
      ["message"]=>
      array(1) {
        [0]=>
        array(2) {
          ["code"]=>
          array(1) {
            [0]=>
            array(0) {
            }
          }
          ["text"]=>
          array(1) {
            [0]=>
            array(0) {
            }
          }
        }
      }
    }
  }

Note all the string nodes have tuned into empty arrays :-/

@judgej
Copy link
Member

judgej commented Jan 31, 2018

I suspect the issue is in the xml2array method, but need to explore a little further to see what is really happening. Between 7.1 and 7.2, this method behaves very differently. Whether it is this method, or the data passed to it, I don't know.

@javabudd
Copy link
Author

javabudd commented Jan 31, 2018

Good find, I'll try poking around as well.

@judgej
Copy link
Member

judgej commented Jan 31, 2018

I won't have time to look until I get home (about 8 hours). I cannot find anything on SO or the docs that indicate SimpleXML has changed in 7.2, except for a few bug fixes. So that may point more towards where the XML is parsed. But I may be completely off there.

judgej added a commit to academe/omnipay-authorizenet that referenced this issue Jan 31, 2018
@judgej
Copy link
Member

judgej commented Feb 4, 2018

There is a PR to play with here. It uses the much simpler json_decode of a json_encode to convert the parsed XML object to an array. This does change the structure of the resulting array slightly. It is failing all travis tests on 7.2 due to deprecated code in phpunit (3.9). I'm sure there will be a way to disable some cs code checks for PHP 7.2 only, or perhaps the travis config is not pointing phpcs at the correct directory to scan - not sure.

fotomerchant pushed a commit to fotomerchant/omnipay-authorizenet that referenced this issue Jul 31, 2018
…orizenet into feature/symfony34

* 'master' of https://github.com/thephpleague/omnipay-authorizenet: (42 commits)
  CVV is optional
  Update README.md
  Update README.md
  Fix cs
  Fix codestyle
  Better travisci
  Update to v3
  Removed a var_dump that escaped.
  Introduce PHP7.2 tests
  Issue thephpleague#105 fix.
  Reduce line length
  Add tests for getAccountType
  Add getAccountType
  Add tests for getCVVCode
  Add getCVVCode
  Correct spacing
  Add capture only to aim gateway
  Update capture only request field order
  Update tests
  Update request based on discussion on issue-93
  ...

# Conflicts:
#	src/Message/AIMAbstractRequest.php
#	src/Message/AIMAuthorizeRequest.php
@judgej
Copy link
Member

judgej commented Dec 31, 2018

A fix has been in place since January 2017 (for version 3.0.0) that changes the way the XML is decoded to an array. That was probably merged in accidentally, but it does not seem to have raised any issues, so I'm going to close this issue.

If I've overlooked anything here, please reopen or raise a new issue if there are other subsequent side-effects. The JSON encode/decode technique used to convert the XML to an array does have its limits (e.g. not properly handling attributes), but the XML supplied by this gateway does not seem to hit it.

@judgej judgej closed this as completed Dec 31, 2018
judgej added a commit that referenced this issue Feb 13, 2019
Fix issues related to xml2Array in query functions

See also issue #105 - the same problem on a different message. Perhaps this flags up refactoring needed to remove some duplicate code.
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

No branches or pull requests

2 participants