Missing filter process callbacks are silently ignored

Would it be better to remove the function_exists() here?

Another option is call_user_func() or call_user_func_array() - those throw warnings if callbacks aren't available rather than fatal, however I'm not convinced this is something that should ever get out of sync so that you couldn't recover from it.

check_markup() is cached 99% of the time, and when it's not, what we do in this particular case is not going to have a measurable difference, the filters themselves are usually extremely costly to run.

Here it is with call_user_func(). This means that if D7 contrib modules or custom code are triggering this, and aren't updated between this getting committed and the next stable release, it'll be a warning instead of 500 when they upgrade.

I'd be OK with just using $function and no function_exists() check in D8, but I'd want to find the original issue that the function_exists() was added first.

Call user func doesn't silently ignore, it throws a php warning. Looking at the original commit though, these just look like registry function_exists() that gave no purpose whatsoever, so let's remove them and use $function straight.

Agreed with #8. Just remove the additional condition, process callbacks are required, and documented as required.

However, I don't see how this is major. Critical/major for a contrib module perhaps, but not for core.

#1059884: Drop function_exists for 'callback' functions globally removed most/all function_exists() throughout core for D8.

New code to be ported:

  // Give filters the chance to escape HTML-like data such as code or formulas.
  foreach ($filters as $name => $filter) {
    if ($filter->status && isset($filter_info[$name]['prepare callback'])) {
      $function = $filter_info[$name]['prepare callback'];
      $text = $function($text, $filter, $format, $langcode, $cache, $cache_id);
    }
  }

  // Perform filtering.
  foreach ($filters as $name => $filter) {
    if ($filter->status && isset($filter_info[$name]['process callback'])) {
      $function = $filter_info[$name]['process callback'];
      $text = $function($text, $filter, $format, $langcode, $cache, $cache_id);
    }
  }

Backported for D7.

Thanks, @avr!

That's sufficient for me. I do not think this needs tests (and, how to test a fatal error in the first place? ;)).

This is a security improvement. But it is also a slight API change in that missing callbacks were silently ignored before, but that's clearly wrong behavior.

This is too risky to commit a couple days before 7.15 is released, so I'm leaving it for now... sorry.

I'm also pretty skeptical about committing this to Drupal 7 at all, though. Fatal errors are good if they occur when someone is developing new code (i.e., it definitely makes sense in Drupal 8). But not so good when they suddenly start occurring on live websites...

My vote would be to reconsider some of the earlier ideas in this issue (watchdog messages, PHP warnings that trigger via call_user_func(), etc). That way we still make noise about the error, but don't bring down a website for what in all likelihood is a not-very-serious problem.

Also note that the function_exists() may have been added in D7, but the actual behavior here (silently ignoring a missing process step) goes way back before that (at least to Drupal 5, and probably earlier). Because it used to be done by invoking the module's hook with a 'process' $op, which the module could easily have ignored.

The branch maintainer says a different approach would be preferred for D7, and I agree this would not be a good thing to backport. It'd be the missing bundle exception all over again. Let's try a different approach?

Edit: also, I checked with a member of the security team and this is not a security issue.