fatal error in ManageMembers.php

[Chen Zhen]

I'm working on something & found an error in the code.

Fatal error: Unsupported operand types

File:
/Sources/ManageMembers.php

find:

Code Select Expand


if ($context['sub_action'] == 'query' && !empty($_REQUEST['params']) && empty($_POST))
$_POST += @unserialize(base64_decode($_REQUEST['params']));


if $_POST is empty then just use equals since it is not adding anything to NULL
...

Code Select Expand


if ($context['sub_action'] == 'query' && !empty($_REQUEST['params']) && empty($_POST))
$_POST = @unserialize(base64_decode($_REQUEST['params']));


test it as such:
../index.php?action=admin;area=viewmembers;sa=query;params=whatever

[Shambles]

Is it complaining that

Code Select Expand

$_POST += @unserialize(base64_decode($_REQUEST['params']));

should be

Code Select Expand

$_POST .= @unserialize(base64_decode($_REQUEST['params']));


?

[emanuele]

Nope, it should be plain and simple equal as suggested by the OP.

[Shambles]

Syntactically tho, the .= is correct I gather?

[emanuele]

No, it is not.

.= concatenates strings.
+= "merge" arrays.
= assigns and remove anything that was there previously.

empty($something) could mean that $something is one of: not set at all, null, false, '' (i.e. empty string), 0 (zero), array() (an empty array).
As far as I know, $_POST should always be initialized at an array (so I'm not sure what the OP is doing, but he may have altered the value of $_POST and that's bad, but cannot be sure).
Though I doubt using "=" is a good thing because if unserialize fails $_POST will become something different from an array (false in particular), and that is bad, I think.

[Chen Zhen]

No, it is not.

.= concatenates strings.
+= "merge" arrays.
= assigns and remove anything that was there previously.

empty($something) could mean that $something is one of: not set at all, null, false, '' (i.e. empty string), 0 (zero), array() (an empty array).
As far as I know, $_POST should always be initialized at an array (so I'm not sure what the OP is doing, but he may have altered the value of $_POST and that's bad, but cannot be sure).
Though I doubt using "=" is a good thing because if unserialize fails $_POST will become something different from an array (false in particular), and that is bad, I think.

emanuele,

The condition asks if empty (NULL) and then attempts to add to its array with += ... If it is empty += is not what is used.
In this case $_POST has a possible array of data from a form or from the url, correct?
The value of $_POST in this case is being altered via the url. What I am getting at is fixing it so no error occurs in any event.
I am assuming of course that it is perhaps the developers intent that php fatal errors do not occur with what is typed in the url (serious statement - not sarcasm. please do not misunderstand.).

Perhaps this is more prudent and does not create an error:

Code Select Expand


if ($context['sub_action'] == 'query' && !empty($_REQUEST['params']) && count($_POST)>0)
$_POST += @unserialize(base64_decode($_REQUEST['params']));
elseif ($context['sub_action'] == 'query' && !empty($_REQUEST['params']))
$_POST = @unserialize(base64_decode($_REQUEST['params']));


or..

Code Select Expand


if ($context['sub_action'] == 'query' && !empty($_REQUEST['params']) && !empty($_POST))
$_POST += @unserialize(base64_decode($_REQUEST['params']));
elseif ($context['sub_action'] == 'query' && !empty($_REQUEST['params']))
$_POST = @unserialize(base64_decode($_REQUEST['params']));


[Arantor]

The condition asks if empty

empty() matches more than null, it matches '', 0 and an array with no elements.

In fact, cleanRequest() in QueryString.php should be preventing the case of $_POST not being an array which means something breaks it between initial definition and that point in time.

[emanuele]

Ahhh...now I got it.
The issue is exactly when unserialize fails, it returns false and the error is raised.

ETA: in 2.1 is already fixed by not assigning anything to $_POST:
https://github.com/SimpleMachines/SMF2.1/blob/master/Sources/ManageMembers.php#L283

[Arantor]

Better question, why does unserialize fail?

[emanuele]

Because when he said "whatever" in the example URL, he was literally implying "whatever"...

ETA: don't ask me why...

[Chen Zhen]

Better question, why does unserialize fail?

Hmm, yes..

Code Select Expand


if ($context['sub_action'] == 'query' && !empty($_REQUEST['params']) && empty($_POST))
$_POST += (array)@unserialize(base64_decode($_REQUEST['params']));


I gather the parameter being added is not an array in this example, (array) causes it to be an array no matter what.

[emanuele]

ETA: in 2.1 is already fixed by not assigning anything to $_POST:
https://github.com/SimpleMachines/SMF2.1/blob/master/Sources/ManageMembers.php#L283