[Bug Fix] Fixing TypeError in Giphy Option Configuration in XenForo 2.3.0

[jman]

Affected version

2.3.0

Content:

Hello XenForo Community,

I encountered a bug related to the Giphy option configuration in XenForo 2.3.0, which results in a TypeError. The error occurs when the callback function in XF\Option\Giphy expects an array but receives a string instead. Here’s the detailed error message:

TypeError: XF\Option\Giphy::XF\Option\{closure}(): Argument #1 ($buttonSet) must be of type array, string given, called in /home/admin/domains/shadowcoders.net/public_html/src/XF/Option/Giphy.php on line 124 in src/XF/Option/Giphy.php at line 57
XF\Option\Giphy::XF\Option\{closure}() in src/XF/Option/Giphy.php at line 124
XF\Option\Giphy::updateToolbarButtons() in src/XF/Option/Giphy.php at line 56
XF\Option\Giphy::insertGiphyToolbarButton() in src/XF/Option/Giphy.php at line 47
XF\Option\Giphy::verifyOption() in src/XF/Entity/Option.php at line 217
XF\Entity\Option->verifyOptionValue() in src/XF/Mvc/Entity/Entity.php at line 836
XF\Mvc\Entity\Entity->_verifyValueCustom() in src/XF/Mvc/Entity/Entity.php at line 677
XF\Mvc\Entity\Entity->set() in src/XF/Mvc/Entity/Entity.php at line 612
XF\Mvc\Entity\Entity->__set() in src/XF/Repository/OptionRepository.php at line 120
XF\Repository\OptionRepository->updateOptions() in src/XF/Admin/Controller/OptionController.php at line 92
XF\Admin\Controller\OptionController->actionUpdate() 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 updateToolbarButtons 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 groupData['buttons'] is always an array before passing it to the callback function.
2. Modify updateToolbarButtons Method: Add a type check for editorToolbarConfig and ensure the callback is called with an array.

Here's the full code with the necessary modifications:

<?php

namespace XF\Option;

use XF\Entity\Option;
use XF\Repository\OptionRepository;

use function is_array;

class Giphy extends AbstractOption
{
    public static function verifyOption(&$value, Option $option)
    {
        if ($option->isInsert())
        {
            return true;
        }

        if (empty($value['enabled']))
        {
            if ($option->option_value['enabled'])
            {
                // just disabled
                static::removeGiphyToolbarButton();
            }

            return true;
        }

        if ($value['enabled'])
        {
            if (empty($value['api_key']))
            {
                $option->error(\XF::phrase('please_enter_value_for_required_field_x', ['field' => 'giphy[api_key]']), $option->option_id);
                return false;
            }

            if (!preg_match('/^[a-z0-9]{32}$/i', $value['api_key']))
            {
                $option->error(\XF::phrase('please_enter_a_valid_api_key'), $option->option_id);
                return false;
            }

            if (!$option->option_value['enabled'])
            {
                // just enabled
                static::insertGiphyToolbarButton();
            }
        }

        return true;
    }

    public static function insertGiphyToolbarButton()
    {
        static::updateToolbarButtons(
            function (array $buttonSet)
            {
                $insertPosition = null;
                foreach ($buttonSet as $k => $button)
                {
                    if ($button == 'xfSmilie')
                    {
                        $insertPosition = $k + 1;
                    }
                    else if ($button == 'xfInsertGif')
                    {
                        // already have it
                        $insertPosition = null;
                        break;
                    }
                }

                if ($insertPosition !== null)
                {
                    array_splice($buttonSet, $insertPosition, 0, ['xfInsertGif']);
                }

                return $buttonSet;
            }
        );
    }

    public static function removeGiphyToolbarButton()
    {
        static::updateToolbarButtons(
            function (array $buttonSet)
            {
                $newButtons = [];

                foreach ($buttonSet as $button)
                {
                    if ($button == 'xfInsertGif')
                    {
                        continue;
                    }

                    $newButtons[] = $button;
                }

                return $newButtons;
            }
        );
    }

    protected static function updateToolbarButtons(callable $buttonsCallback)
    {
        $toolbarButtons = \XF::options()->editorToolbarConfig;

        if (!is_array($toolbarButtons)) {
            throw new \InvalidArgumentException('Expected array for editorToolbarConfig, got ' . gettype($toolbarButtons));
        }

        foreach ($toolbarButtons as $type => &$group)
        {
            if (!is_array($group))
            {
                continue;
            }

            foreach ($group as &$groupData)
            {
                if (!is_array($groupData) || empty($groupData['buttons']))
                {
                    continue;
                }

                if (!is_array($groupData['buttons']))
                {
                    // Skip if buttons are not in array form
                    continue;
                }

                $groupData['buttons'] = $buttonsCallback($groupData['buttons']);
            }
        }

        \XF::repository(OptionRepository::class)->updateOption('editorToolbarConfig', $toolbarButtons);
    }
}

Explanation of Changes:
1. verifyOption Method Signature: Ensured that verifyOption conforms to the expected usage in XenForo.
2. Type Check for editorToolbarConfig: Added a type check for editorToolbarConfig in updateToolbarButtons to ensure it is always an array.
3. Ensuring Array Structure: Added a check to ensure $groupData['buttons'] is an array before passing it to the callback function.

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

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