[Bug Fix] Fixing TypeError in ButtonManagerController in XenForo 2.3.0

[jman]

Affected version

2.3.0

Content:

Hello XenForo Community,

I encountered a bug related to the ButtonManagerController in XenForo 2.3.0, which results in a TypeError. The error occurs when the removeOrphanedButtons method expects an array but receives a string instead. Here’s the detailed error message:

ErrorException: [E_WARNING] foreach() argument must be of type array|object, string given in src/XF/Admin/Controller/ButtonManagerController.php at line 112
XF::handlePhpError() in src/XF/Admin/Controller/ButtonManagerController.php at line 112
XF\Admin\Controller\ButtonManagerController->removeOrphanedButtons() in src/XF/Admin/Controller/ButtonManagerController.php at line 63
XF\Admin\Controller\ButtonManagerController->actionEdit() in src/XF/Mvc/Dispatcher.php at line 362
XF\Mvc\Dispatcher->dispatchClass() in src/XF/Mvc/Dispatcher.php at line 264
XF\Mvc\Dispatcher->dispatchFromMatch() in src/XF/Mvc/Dispatcher.php at line 121
XF\Mvc\Dispatcher->dispatchLoop() in src/XF/Mvc/Dispatcher.php at line 63
XF\Mvc\Dispatcher->run() in src/XF/App.php at line 2777
XF\App->run() in src/XF.php at line 798
XF::runApp() in admin.php at line 15

The issue arises because the removeOrphanedButtons method processes toolbarButtons and calls the callback with each groupData['buttons'], which might not always be an array.

Steps to Fix:

1. Check the Data Structure: Ensure that toolbarButtons is always an array before passing it to the removeOrphanedButtons method.
2. Modify removeOrphanedButtons Method: Add a type check for toolbarButtons and ensure the callback is called with an array.

Here's the full code with the necessary modifications:

<?php

namespace XF\Admin\Controller;

use XF\Mvc\Dispatcher;
use XF\Mvc\ParameterBag;

class ButtonManagerController extends \XF\Admin\Controller\AbstractController
{
    public function actionEdit(ParameterBag $params)
    {
        $toolbarButtons = $this->getToolbarButtons(); // Ensure this method returns an array
        $availableButtons = $this->getAvailableButtons(); // Ensure this method returns an array

        if (!is_array($toolbarButtons)) {
            $toolbarButtons = []; // Initialize as an empty array if not an array
        }

        $this->removeOrphanedButtons($toolbarButtons, $availableButtons);

        // Continue with the rest of the function
        // ...

        return $this->view('XF:ButtonManager\Edit', 'button_manager_edit', [
            'toolbarButtons' => $toolbarButtons,
            'availableButtons' => $availableButtons
        ]);
    }

    protected function getToolbarButtons()
    {
        // Logic to get toolbar buttons, ensure this returns an array
        // Example:
        return [
            'group1' => [
                'buttons' => ['button1', 'button2']
            ],
            'group2' => [
                'buttons' => ['button3', 'button4']
            ]
        ];
    }

    protected function getAvailableButtons()
    {
        // Logic to get available buttons, ensure this returns an array
        // Example:
        return [
            'button1' => 'Button 1',
            'button2' => 'Button 2',
            'button3' => 'Button 3'
        ];
    }

    protected function removeOrphanedButtons(array &$toolbarButtons, array $availableButtons)
    {
        foreach ($toolbarButtons as $groupId => $group)
        {
            if (!is_array($group) || !isset($group['buttons']) || !is_array($group['buttons'])) {
                continue; // Skip groups that don't have the expected structure
            }

            foreach ($group['buttons'] as $index => $button)
            {
                if (!array_key_exists($button, $availableButtons))
                {
                    unset($toolbarButtons[$groupId]['buttons'][$index]);
                }
            }
        }
    }
}

Explanation of Changes:
1. Type Check for toolbarButtons: Added a type check for toolbarButtons in actionEdit to ensure it is always an array.
2. Ensuring Array Structure: Added a check to ensure $groupData['buttons'] is an array before processing it in removeOrphanedButtons.

By applying these changes, you can avoid the TypeError and ensure the removeOrphanedButtons method processes the toolbar buttons correctly.

I hope this helps! Feel free to ask any questions or provide further feedback.