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

PHP WARNING: "Argument 2 passed to elgg_extract() must be of the type array, null given #9772

Closed
rohit1290 opened this Issue Apr 25, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@rohit1290

rohit1290 commented Apr 25, 2016

While doing my monthly error log review, I noted that a particular warning was appearing again and again i.e., PHP WARNING: "Argument 2 passed to elgg_extract() must be of the type array, null given, called in /path/to/engine/lib/comments.php on line 284 and defined" in file /path/to/engine/lib/elgglib.php (line 1256)

I saw that there is a check that if the $returnvalue parameter is not an array then it will not allow us to proceed. But what if the array was empty/null? The above PHP warning is generated if a blank or null array is passed. I know this is only a warning but I thought that it would be nice to get this fixed using the below code:

if (!is_array($returnvalue) || $returnvalue == null || $returnvalue == "" ) {
    // another hook handler returned a non-array, let's not override it
    return;
}

Rest its up to the core team to decide.

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Apr 25, 2016

Contributor

What version of Elgg are you using?

A similar (the same?) issue has been reported in #8333. It has been fixed in Elgg 1.11.3 (and should also no longer occur on Elgg versions newer than 1.11.3). Elgg versions older than 1.12 don't get any bugfixes anymore at the present time though.

Contributor

iionly commented Apr 25, 2016

What version of Elgg are you using?

A similar (the same?) issue has been reported in #8333. It has been fixed in Elgg 1.11.3 (and should also no longer occur on Elgg versions newer than 1.11.3). Elgg versions older than 1.12 don't get any bugfixes anymore at the present time though.

@rohit1290

This comment has been minimized.

Show comment
Hide comment
@rohit1290

rohit1290 Apr 25, 2016

I am using Elgg 1.12

rohit1290 commented Apr 25, 2016

I am using Elgg 1.12

@rohit1290

This comment has been minimized.

Show comment
Hide comment
@rohit1290

rohit1290 Apr 25, 2016

Yes, I have seen that issue and the fix was to check that it was an array. But what if a blank array is passed. it will bypass the check as it is an array but still generated the warning as it's a null or blank array.

rohit1290 commented Apr 25, 2016

Yes, I have seen that issue and the fix was to check that it was an array. But what if a blank array is passed. it will bypass the check as it is an array but still generated the warning as it's a null or blank array.

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 Apr 25, 2016

Member

Are you sure an empty array generates that message? I don't think that's correct, I think it means something is passing a literal NULL, not array()

Member

beck24 commented Apr 25, 2016

Are you sure an empty array generates that message? I don't think that's correct, I think it means something is passing a literal NULL, not array()

@rohit1290

This comment has been minimized.

Show comment
Hide comment
@rohit1290

rohit1290 Apr 26, 2016

if it was a null, then it wud have get filtered out in d current check where null is not an array.

i did a negative testing by creating a blank array and saw that it was bypassing the current check defined in the function. And that is why i came up with the solution that it should also check for null/blank array.

rohit1290 commented Apr 26, 2016

if it was a null, then it wud have get filtered out in d current check where null is not an array.

i did a negative testing by creating a blank array and saw that it was bypassing the current check defined in the function. And that is why i came up with the solution that it should also check for null/blank array.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Apr 26, 2016

Contributor

PR #9775. It has nothing to do with elgg_extract() rather the hook not validating the values properly.

Contributor

hypeJunction commented Apr 26, 2016

PR #9775. It has nothing to do with elgg_extract() rather the hook not validating the values properly.

@rohit1290

This comment has been minimized.

Show comment
Hide comment
@rohit1290

rohit1290 Apr 27, 2016

I have been monitoring my error log for the past 12 hours and the said PHP warning has stop appearing in my error log. hypeJunction solution did the magic.

rohit1290 commented Apr 27, 2016

I have been monitoring my error log for the past 12 hours and the said PHP warning has stop appearing in my error log. hypeJunction solution did the magic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment