Problem/Motivation
On PHP 8+, visiting the Imagecache External settings form triggers a fatal error:
TypeError: implode(): Argument #1 ($array) must be of type array, string given in implode() (line 180 of modules/contrib/imagecache_external/src/Form/SettingsForm.php).
The offending lines are the #default_value for the “Allowed tags” and "Allowed attributes" textfields:
'#default_value' => implode(' ', $config->get('svg_settings.allowed_tags')),
'#default_value' => implode(' ', $config->get('svg_settings.allowed_attributes')),
$config->get('svg_settings.allowed_tags') can return a string (or null), but implode() now requires an array in PHP 8, so a TypeError is thrown when the settings form is built.
This prevents administrators from accessing the Imagecache External settings page when running PHP 8+.
Steps to reproduce
Navigate to the Imagecache External settings form (e.g. at /admin/config/media/imagecache-external).
The page fails to load and the error is thrown.
Proposed resolution
Ensure that the #default_value for the “Allowed tags” and "Allowed attributes" textfields are always a string, while still supporting both array and string configuration values for backward compatibility.
Replace the existing lines:
'#default_value' => implode(' ', $config->get('svg_settings.allowed_tags')),
with something like:
$allowed_tags = $config->get('svg_settings.allowed_tags');
$form['allowed_tags'] = [
'#type' => 'textfield',
'#title' => $this->t('Allowed tags'),
'#description' => $this->t('Space separated list of custom tags that should be added to the list of allowed tags'),
'#default_value' => is_array($allowed_tags) ? implode(' ', $allowed_tags) : (string) $allowed_tags,
];If svg_settings.allowed_tags is stored as an array, it is imploded into a space-separated string for the textfield, preserving the original intention.
If it is already a string (or null), it is cast to a string without calling implode(), avoiding the PHP 8 TypeError.
This change is backward-compatible with existing configurations and does not alter how the value is displayed to the user.
Remaining tasks
Confirm that there are no other usages of implode() on config values that may also return strings.
Optionally, add a simple test to ensure the settings form can be built on PHP 8+ without errors.
User interface changes
None
API changes
None
Data model changes
None
Issue fork imagecache_external-3566491
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3.0.x
changes, plain diff MR !19
- 3566491-typeerror-implode-argument
changes, plain diff MR !18
Comments
Comment #5
jillh68 commentedComment #6
jillh68 commentedComment #7
swentel commentedHmm, makes sense I guess, I'm just wondering how it can't be an array? Did the update function (imagecache_external_update_8105()) fail then and not store anything for these properties? Or is there another way to reproduce this, because I currently can't ?
Also, there are two additional spaces in the patch for the $form arrays, that's not necessary.
Comment #8
jillh68 commentedYes, looks like the fix is in update 8105 ( ->set('svg_settings', ['allowed_tags' => [], 'allowed_attributes' => []]).
It must not have run on the site I was testing. Running the update on a new site fixed the issue.
Comment #10
swentel commented