gggeek / phpxmlrpc

A php library for building xmlrpc clients and servers
http://gggeek.github.io/phpxmlrpc/
Other
229 stars 94 forks source link

Remove warnings in PHP 8.1 #104

Closed rolandseuhs closed 1 year ago

rolandseuhs commented 1 year ago

Please add these small changes to avoid warnings in PHP8:

  1. src/Request.php line 282:

    $respEncoding = XMLParser::guessEncoding(@($this->httpResponse['headers']['content-type'] ?? ""), $data);
  2. src/Helper/XMLParser.php line 637:

    if (preg_match('/;\s*charset\s*=([^;]+)/i', $httpHeader ?? "", $matches)) {
  3. src/Value.php line 410:

    return $this->me['struct'][$key] ?? null;
  4. src/Value.php line 482:

    return $this->me['array'][$key] ?? null;
gggeek commented 1 year ago

Hello.

I am not sure about the proposed changes for src/Value: accessing a non-existing member of a struct/array value should imho raise an error message. This is not an issue related to php version, but rather to the API of the library - a warning would be generated in the same scenarios even when running on php 5.3. You should have methods available allowing to check for a member's existence before using it, such as structmemexists (deprecated) or even plain isset.

rolandseuhs commented 1 year ago

Hi,

I use this code now and the warning appears in the if-statement:

    if ($obj->structmem($key))
    {   d("has_structmem\n");
        foreach($obj->structmem($key) as $t)
        {   return $t;
        }
    }

when I use

    if ($obj->structmemexists($key))

the warning goes away, but when that is deprecated, how should I use isset?

gggeek commented 1 year ago

As for points 1 and 2, I think that those are trying to fix the same issue, namely the case of $this->httpResponse['headers']['content-type'] being missing. In that case, the warning for accessing a non-existing array member is already silenced by usage of the @ operator, but then a call is executed to guessEncoding passing in a NULL, which results in a warning. I think that it is thus probably ok to fix only the code in Request.php so that it always passes a string value to guessEncoding. If the developers do call guessEncoding from their own code, it is their responsibility to pass in a value of the appropriate type (enforcing strict types seems to be a trend in the php community these days). What do you think?

gggeek commented 1 year ago

About usage of isset(): the Value class does expose the ArrayAccess interface, meaning it can be treated as an array (that works well of course when you know that the given value is either an xmlrpc array or struct).

This should be code equivalent to your sample above:

// quick way to create non-scalar Value objects
$e = new \PhpXmlRpc\Encoder();
$obj = $e->encode(['hello' => ['john', 'judy']]);

if (isset($obj['hello'])) {
    foreach($obj['hello'] as $val) {
        //echo $val->scalarval() . "\n";
        return $val;
    }
}

If all you are interested in is the first element of the array which seems to be the expected type for the $key element, it is also possible to do away with the foreach and replace it with a reset call, which seems a better fit (even though it results in the same amount of lines of code)

if (isset($obj['hello'])) {
    $v1 = iterator_to_array($obj['hello']);
    //var_dump(reset($v1));
    return reset($v1);
}

There might have performance implications though, if you are dealing with very large xmlrpc values

rolandseuhs commented 1 year ago

That is really strange.

With if ($obj->structmem($key)) I get 7 warnings

With isset($obj[$key]) I get 3 warnings

With if ($obj->structmemexists($key)) I get no warnings

Please un-deprecate structmemexists :-)

gggeek commented 1 year ago

That is weird, indeed.

Would you be able to give me more information, to try to reproduce the warnings you get with "isset"?

a var_dump / var_export of $obj would do, or as a copy of the xml being parsed, or if the data is coming from a publicly accessible server, a code snippet to retrieve it via an xmlrpc call.

If the data is sensitive or large, we could share it by mail, google drive, private gist or any other private channel of your preference.

rolandseuhs commented 1 year ago

Sorry, my mistake, the 3 warnings from isset($obj[$key]) - and 3 of the 7 from if ($obj->structmem($key)) were coming from somewhere else where I called structmem directly.

So it works, thanks for the support

gggeek commented 1 year ago

You are welcome.

I will close this ticket, as a fix for the warning with guessEncoding has been applied, and will be part of the next release