WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#25672 closed defect (bug) (fixed)

is_a() causing errors with get_users()

Reported by: dgdax Owned by: wonderboymusic
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.6.1
Component: General Keywords: has-patch 4.2-early
Focuses: Cc:

Description (last modified by SergeyBiryukov)

(copy of post on support forum : http://wordpress.org/support/topic/is_a-causing-errors-with-get_users?replies=2#post-4795246 )

Running WordPress with Yii framework sub-site.
Use of is_a() function in capabilities.php (line 486) is causing errors, since causes instantiation of a WP_User object - which is unknown to Yii.

is_a() was deprecated in PHP 5.0.0 - but did not cause this problem until PHP 5.3.7, when its behaviour changed: if first argument is not an object, __autoload() is triggered. (see php manual: http://www.php.net/manual/en/function.is-a.php)

Example: error if I use get_users() with the 'all_with_meta' option.

Cause: is_a() called with non-object ($id)

This is at least inefficient - and in my situation fatal! Even when it works, this generates unnecessary log warnings in many cases (see all the other posts)

Solution:
replace:

if ( is_a( $id, 'WP_User' ))

with:

if ( $id instanceof WP_User )

... or equivalent, wherever is_a() may be called with a non-object.

Please can you fix this for increased compatibility in future versions - both with PHP and all the other plugins!

Attachments (4)

25672.diff (24.4 KB) - added by markoheijnen 6 years ago.
25672.2.diff (24.8 KB) - added by markoheijnen 6 years ago.
25672.3.diff (25.2 KB) - added by markoheijnen 6 years ago.
25672.4.diff (23.9 KB) - added by wonderboymusic 5 years ago.

Download all attachments as: .zip

Change History (25)

#1 @markoheijnen
6 years ago

It wasn't deprecated in 5.3 anymore. I do have a patch somewhere that fixes all the is_a() to instanceof

@markoheijnen
6 years ago

#3 @SergeyBiryukov
6 years ago

  • Component changed from General to Warnings/Notices
  • Description modified (diff)
  • Summary changed from WordPress › Support » How-To and Troubleshooting is_a() causing errors with get_users() to is_a() causing errors with get_users()

#4 @johnbillion
6 years ago

I believe is_a() is the preferred method due to errors that can arise with instanceof negation. Example:

Correct:

if ( ! is_a( $user, 'WP_User' ) )

Incorrect, but not obviously so:

if ( ! $user instanceof 'WP_User' )

The correct syntax for negation when using instanceof is:

if ( ! ( $user instanceof 'WP_User' ) )

#5 @markoheijnen
6 years ago

Not sure if is_a() is the preferred method since instanceof is faster. How much depends on which PHP version you are using.

#6 @dgdax
6 years ago

Glad to see this is generating some discussion...
Some additional comments/reactions to the above:

  • thanks to #Sergey for cleaning up my description
  • is_a() does work in PHP 5.3+, but behaviour changed since v5.3.7 - and this is what is causing the problem.
  • instanceof appears to be the 'officially' preferred method, and as #marko points out, there are performance benefits also (because no need for instantiation?). Apparently in some situations where instanceof can also cause instantiation, but I believe these are rare.
  • I think the negation issue is really just a warning to developers to be careful when changing the code. The addition of the brackets ( ... ) in the corrected example is simply good coding practice, as it makes the intention explicit, both to PHP and to the next developer reading the code. (for comparison, would you be so surprised if ( ! $user1 == $user2 ) gave a different result to ( $user1 !== $user2 ) ? )

#7 @markoheijnen
6 years ago

the changed behavior was that that autoload got triggered. That was fixed in 5.3.9. Not sure if that bug was ever an issue in WordPress.

#8 @nacin
6 years ago

  • Component changed from Warnings/Notices to General
  • Keywords 2nd-opinion added

@markoheijnen
6 years ago

@markoheijnen
6 years ago

#9 @markoheijnen
6 years ago

Added two patches: 25672.2.diff is a refresh and 25672.3.diff adds another one I missed and was mentioned in #29139. Unsure what the next steps are.

#10 @markoheijnen
6 years ago

#29139 was marked as a duplicate.

#11 @markoheijnen
6 years ago

  • Keywords has-patch added; needs-patch removed

#12 @wonderboymusic
6 years ago

  • Milestone changed from Awaiting Review to 4.1

! ( $marko instanceof Marko ) is totally doable as a negation standard for us.

I will wait for another dose of @nacin-like feedback before proceeding, but I champion improvements like these.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


5 years ago

#14 @helen
5 years ago

  • Milestone changed from 4.1 to Future Release

Not new, no feedback, punting.

#15 @markoheijnen
5 years ago

What needs to be done to get this into 4.2?

#16 @SergeyBiryukov
5 years ago

  • Keywords 4.2-early added

#17 @iseulde
5 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

#18 @wonderboymusic
5 years ago

  • Keywords 2nd-opinion removed

25672.4.diff doesn't touch 3rd party libraries and adds some braces around some unfortunately-spaced if/elses

#19 @wonderboymusic
5 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 31188:

In PHP 5.0.0, is_a() became deprecated in favour of the instanceof operator. Calling is_a() would result in an E_STRICT warning.

In PHP 5.3.0, is_a() is no longer deprecated, and will therefore no longer throw E_STRICT warnings.

To avoid warnings in PHP < 5.3.0, convert all is_a() calls to $var instanceof WP_Class calls.

instanceof does not throw any error if the variable being tested is not an object, it simply returns false.

Props markoheijnen, wonderboymusic.
Fixes #25672.

#20 @nacin
5 years ago

Does this mean we should never add another is_a() to core? Who or what is enforcing that decision? It seems arbitrary. We must consider the added cost during implementation of new code in cases like this.

As noted in this thread: is_a() and instanceof have slightly different behaviors, and negating instanceof is also weird (you have to use parentheses and such).

I think that anyone who is looking for E_STRICT in PHP 5.2 deserves to have a full error log.

Also, if you're using 5.3.7 or 5.3.8, least you can do is update to something secure. (Very few WP sites run these two versions.)

#21 @nacin
5 years ago

@wonderboymusic @ markoheijnen — super minor, just curious what your feedback is for the above comment.

Note: See TracTickets for help on using tickets.