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

Command icon 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:

Comments

jillh68 created an issue. See original summary.

jillh68’s picture

Issue summary: View changes
jillh68’s picture

Issue summary: View changes
swentel’s picture

Status: Active » Postponed (maintainer needs more info)

Hmm, 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.

jillh68’s picture

Yes, 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.

swentel’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.