Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Categories controller spoofing it's name is a potential source of bugs

The Categories Controller (/applications/vanilla/controllers/class.categoriescontroller.php) spoofs it's name to appear as "DiscussionsController":

Line 93:

$this->ControllerName = 'DiscussionsController';

Line 303:

// Change the controller name so that it knows to grab the discussion views
$this->ControllerName = 'DiscussionsController';

Many plugins legitly check the ControllerName property of the current controller to decide whether to run or not. Behaviour will be incorrect when only one of "discussionscontroller" and "categoriescontroller" is present.

Is this spoofing really necessary, or is it just a dirty hack? If it can't be avoided, developers should take it into account when coding their plugins and identify the controller by other means, such as $Controller->ClassName or get_class($Sender).

By the way, the name should be spoofed in lower case ("discussionscontroller"), like all other controller names.

Comments

  • businessdadbusinessdad Stealth contributor MVP
    edited June 2015

    That "trick" can be used to fire some other controller's events, and it can be handy. However, its abuse could indeed cause issues, especially if type hinting is used.

    Example

    $this->ControllerName = 'DiscussionsController';
    $this->FireEvent('SomeEvent');
    
    [...]
    
    /**
     * Handle the event. The $Sender parameter is specified with a type hint. If a controller
     * other than DiscussionsController is passed, bad things happen.
     */
    public function DiscussionsController_SomeEvent_Handler(DiscussionsController $Sender) {
      // Some stuff
    }
    
    
  • BleistivtBleistivt Moderator
    edited June 2015

    The controllername property is incosistent throughout.

    Buy it is not the preferred way to check for the controller name. Use events specific to the controller (Base_* hooks should be used sparsely anyway) or use InSection().

  • hgtonighthgtonight ∞ · New Moderator
    $this->FireAs('DiscussionsController')->FireEvent('SomeEvent');
    

    No permanent spoofing necessary.

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

Sign In or Register to comment.