Problem/Motivation

template_preprocess_image_formatter() could use mb_strlen() on NULL title

Steps to reproduce

Run core/modules/image/tests/src/Kernel/ImageFormatterTest.php on PHP 8.1

  2x: mb_strlen(): Passing null to parameter #1 ($string) of type string is deprecated
    2x in ImageFormatterTest::testImageFormatterSvg from Drupal\Tests\image\Kernel

Proposed resolution

Check if NULL passed

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3240906-2.patch584 bytesandypost
#4 3240906-5.patch490 bytesandypost
#4 interdiff.txt516 bytesandypost
#2 3240906-2.patch584 bytesandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 3240906-2.patch584 bytes

Patch from https://git.drupalcode.org/project/drupal/-/commit/2efec5942bd7f5a885234...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work 
+++ b/core/modules/image/image.field.inc
@@ -58,7 +58,7 @@ function template_preprocess_image_formatter(&$variables) {
-  if (mb_strlen($item->title) != 0) {
+  if (!is_null($item->title) && mb_strlen($item->title) != 0) {

I think this could be better written as: if (!empty(item->title)) {

Should have the same effect as far as I can see.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt516 bytes
new 3240906-5.patch490 bytes

makes sense

mondrake’s picture

mondrake
Location 🇮🇹
commented

but a title could be legitimately “0”, and that would be considered empty and therefore skipped, no?

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 3240906-2.patch584 bytes

@mondrake thank you! it's valid reason - re-upload patch #2

andypost’s picture

andypost
he/him
Primary language Russian
commented

otoh it means we have no test coverage for "0" in titles

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@mondrake++

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

  • larowlan committed 15e1c65 on 9.3.x 
    Issue #3240906 by andypost, alexpott, mondrake:...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed 15e1c65 and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.