WP_Widget_Factory does not validate that widget class is a WP_Widget

[carlalexander]

I have been working on an ​article that uses the Widget API. As I was reviewing the code, it came to my attention that you have no validation in WP_Widget_Factory. You can register a widget class that isn't a WP_Widget.

This isn't a problem by itself, but the _register_widgets method calls the WP_Widget _register method. If someone registered a non-WP_Widget class, it will cause a fatal error.

A possible fix would be to add validation in the register function. Like this:

class WP_Widget_Factory {
    // ...

    function register($widget_class) {
        $widget_obj = new $widget_class();

        if ( !is_a($widget_obj, 'WP_Widget') )
            return;

        $this->widgets[$widget_class] = $widget_obj;
    }

    // ...
}

[kwight]

28158.diff is a patch based on the above suggestion.

[helen]

We don't typically patch over developer errors like this. Why wouldn't you want to know why a widget isn't working?

[carlalexander]

Why don't you patch developer errors?

[DrewAPicture]

Replying to carlalexander:

Why don't you patch developer errors?

I think the point is that it's understood that WP_Widget_Factory expects a WP_Widget object here. And in choosing not to pass one, the fatal that gets tossed is the notification that there is developer error. This not really something you'd want fail silently, imo.

[carlalexander]

Replying to DrewAPicture:

I think the point is that it's understood that WP_Widget_Factory expects a WP_Widget object here. And in choosing not to pass one, the fatal that gets tossed is the notification that there is developer error. This not really something you'd want fail silently, imo.

This doesn't match the behaviour in the_widget which does fail silently. If anything, the silent validation in the_widget is not even worth anything since you're going to have had a fatal error before it even reaches that code.

[Denis-de-Bernardy]

Replying to carlalexander:

Replying to DrewAPicture:

I think the point is that it's understood that WP_Widget_Factory expects a WP_Widget object here. And in choosing not to pass one, the fatal that gets tossed is the notification that there is developer error. This not really something you'd want fail silently, imo.

This doesn't match the behaviour in the_widget which does fail silently. If anything, the silent validation in the_widget is not even worth anything since you're going to have had a fatal error before it even reaches that code.

As I recollect, the_widget(), when it was introduced, was designed in such a way that it wouldn't cause a fatal error when a call is left lying around in a theme or in php content while the plugin that introduced the widget is disabled -- which is an end-user error, rather than a developer error. The widget factory, in contrast, should only ever get called when a theme or plugin introduces a new widget.