WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#28158 closed defect (bug) (wontfix)

WP_Widget_Factory does not validate that widget class is a WP_Widget

Reported by: carlalexander Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.9
Component: Widgets Keywords:
Focuses: Cc:

Description

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;
    }

    // ...
}

Attachments (1)

28158.diff (514 bytes) - added by kwight 5 years ago.

Download all attachments as: .zip

Change History (7)

@kwight
5 years ago

#1 @kwight
5 years ago

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

#2 @helen
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

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

#3 follow-up: @carlalexander
5 years ago

Why don't you patch developer errors?

#4 in reply to: ↑ 3 ; follow-up: @DrewAPicture
5 years ago

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.

#5 in reply to: ↑ 4 ; follow-up: @carlalexander
5 years ago

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.

#6 in reply to: ↑ 5 @Denis-de-Bernardy
5 years ago

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.

Last edited 5 years ago by Denis-de-Bernardy (previous) (diff)
Note: See TracTickets for help on using tickets.