Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#19417 closed defect (bug) (fixed)

has_filter() can return 0, and remove_filter() doesn't use $accepted_args

Reported by: nacin Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch commit early
Focuses: Cc:


has_filter() returns priority, which means it can return 0. But core doesn't check for it being identical to false. We should audit all has_filter() calls and make sure 'false !==' or 'false ===' depending on what we want.

Additionally remove_filter() has an $accepted_args argument, but doesn't use it. We should eliminate it from the function definition and docs.

Attachments (2)

19417.diff (4.5 KB) - added by nacin 10 years ago.
19417.refresh.diff (6.6 KB) - added by georgestephanis 10 years ago.

Download all attachments as: .zip

Change History (9)

10 years ago

#1 @nacin
10 years ago

  • Keywords has-patch commit added

We do currently check false === has_filter() in wp-login.php.

#2 @ryan
10 years ago

  • Keywords early added
  • Milestone changed from Awaiting Review to Future Release

Oh, okay.

#3 @georgestephanis
10 years ago

I'm refreshing the ticket now, it didn't apply cleanly -- we really should do this for has_action as well. Adding that in.

#4 @SergeyBiryukov
10 years ago

  • Milestone changed from Future Release to 3.5

#5 @nacin
10 years ago

So, has_filter() and has_action() only returns the priority when $function_to_check is also passed. Otherwise, if you're just checking the hook, it returns a boolean.

In core, we never use the second argument. So, there's no need for us to be careful about its use. This is nothing more than a documentation issue.

#6 @georgestephanis
10 years ago

In which case can we just include a doing_it_wrong() call or the like when some third-party code tries to pass something as priority zero? We're leaving a (small, but existant) hole open for people to do things that could lead to our checks in core returning an untrue result.

EDIT: Ignore the above, I just reread the code and found the short-circuit for false.

Last edited 10 years ago by georgestephanis (previous) (diff)

#7 @nacin
10 years ago

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

In [21288]:

Clarify the return value of has_filter() and has_action(). It returns a boolean if only the first argument is specified. If the second argument is specified, it returns false or an integer, which means it may return a non-boolean value that evaluates to false (so, 0), so you should take care to use the === operator.

Correct the function definition of remove_filter() and remove_action(), which 'accepted' an $accepted_args argument, but did not require or use it.

fixes #19417.

Note: See TracTickets for help on using tickets.