Drupal\Core\Controller\TitleResolver::getTitle() can result in PHP 8.1 deprecation notices due to NULL being set as default

Created on 26 April 2023, almost 2 years ago

Problem/Motivation

Much like #3236789: Drupal\Core\Controller\TitleResolver::getTitle() can result in PHP 8.1 deprecation notices due to NULL being set as placeholder values β†’ , TitleResolver triggers deprecations in PHP 8.1 when a default is not set.

Steps to reproduce

  • define a route without a title
  • navigate to the URL for that route

Proposed resolution

In getTitle default the title to '' if it is NULL

Remaining tasks

Review patch.

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

πŸ› Bug report
Status

Active

Version

9.5

Component
RoutingΒ  β†’

Last updated 1 day ago

Created by

πŸ‡«πŸ‡·France fgm Paris, France

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @fgm
  • πŸ‡«πŸ‡·France fgm Paris, France
  • Status changed to Needs review almost 2 years ago
  • last update almost 2 years ago
    30,322 pass
  • πŸ‡«πŸ‡·France fgm Paris, France

    Suggested patch

  • πŸ‡«πŸ‡·France fgm Paris, France

    Another solution could be to modify hasDefault to use !empty or isset instead of array_key_exists but that seems more intrusive than the suggested patch.

  • Status changed to Needs work almost 2 years ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Don't think there is an issue with the current solution. But a test case showing the problem will be needed.

  • πŸ‡«πŸ‡·France fgm Paris, France

    AFAICS the current implementation is just wrong: it uses strlen on the result of a getDefault, which may be NULL because it only checks that the key exists, not that the resulting value is not empty; and strlen(NULL) is deprecated since PHP 8.1, so any configuration where that function fetches something which can be NULL will return NULL, and any place where that result is used as the argument for strlen can now trigger that deprecation warning.

  • πŸ‡«πŸ‡·France fgm Paris, France
  • πŸ‡ΊπŸ‡¦Ukraine vselivanov Kyiv, Ukraine

    fgm, thank you for the patch.
    It fixed 2 warnings on the node edit page with Drupal 9.5.9.

    It's hard to define the reason of these warnings. Site uses a lot of contrib modules and custom alterations. Even on this node edit page.
    One of them because of token replacements:

    Deprecated function: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in Drupal\Core\Controller\TitleResolver->getTitle() (line 60 of /var/www/docroot/core/lib/Drupal/Core/Controller/TitleResolver.php)
    #0 /var/www/docroot/core/includes/bootstrap.inc(347): _drupal_error_handler_real(8192, 'strlen(): Passi...', '/var/www/docroo...', 60)
    #1 [internal function]: _drupal_error_handler(8192, 'strlen(): Passi...', '/var/www/docroo...', 60)
    #2 /var/www/docroot/core/lib/Drupal/Core/Controller/TitleResolver.php(60): strlen(NULL)
    #3 /var/www/docroot/modules/contrib/token/token.tokens.inc(843): Drupal\Core\Controller\TitleResolver->getTitle(Object(Symfony\Component\HttpFoundation\Request), Object(Symfony\Component\Routing\Route))
    #4 [internal function]: token_tokens('current-page', Array, Array, Array, Object(Drupal\Core\Render\BubbleableMetadata))
    #5 /var/www/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(426): call_user_func_array(Object(Closure), Array)
    #6 /var/www/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(405): Drupal\Core\Extension\ModuleHandler->Drupal\Core\Extension\{closure}(Object(Closure), 'token')
    #7 /var/www/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(433): Drupal\Core\Extension\ModuleHandler->invokeAllWith('tokens', Object(Closure))
    #8 /var/www/docroot/core/lib/Drupal/Core/Utility/Token.php(359): Drupal\Core\Extension\ModuleHandler->invokeAll('tokens', Array)
    #9 /var/www/docroot/core/lib/Drupal/Core/Utility/Token.php(241): Drupal\Core\Utility\Token->generate('current-page', Array, Array, Array, Object(Drupal\Core\Render\BubbleableMetadata))
    #10 /var/www/docroot/core/lib/Drupal/Core/Utility/Token.php(191): Drupal\Core\Utility\Token->doReplace(true, '[current-page:t...', Array, Array, Object(Drupal\Core\Render\BubbleableMetadata))
    #11 /var/www/docroot/modules/contrib/metatag/src/MetatagToken.php(66): Drupal\Core\Utility\Token->replace('[current-page:t...', Array, Array, NULL)
    #12 /var/www/docroot/modules/contrib/metatag/src/MetatagManager.php(771): Drupal\metatag\MetatagToken->replace('[current-page:t...', Array, Array)
    #13 /var/www/docroot/modules/contrib/metatag/src/MetatagManager.php(617): Drupal\metatag\MetatagManager->processTagValue(Object(Drupal\metatag\Plugin\metatag\Tag\Title), Array, Array, false, 'en')
    #14 /var/www/docroot/modules/contrib/metatag/src/MetatagManager.php(549): Drupal\metatag\MetatagManager->generateRawElements(Array, NULL)
    #15 /var/www/docroot/modules/contrib/metatag/metatag.module(521): Drupal\metatag\MetatagManager->generateElements(Array, NULL)
    #16 /var/www/docroot/modules/contrib/metatag/metatag.module(130): metatag_get_tags_from_route()
    #17 /var/www/docroot/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(315): metatag_page_attachments(Array)
    #18 /var/www/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(405): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}(Object(Closure), 'metatag')
    #19 /var/www/docroot/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(316): Drupal\Core\Extension\ModuleHandler->invokeAllWith('page_attachment...', Object(Closure))
    #20 /var/www/docroot/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(289): Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array)
    #21 /var/www/docroot/core/lib/Drupal/Core/Render/Renderer.php(580): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
    #22 /var/www/docroot/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(290): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
    #23 /var/www/docroot/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(132): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
  • πŸ‡«πŸ‡·France fgm Paris, France

    Glad to see I'm not alone in this. Since you identified the problem as token-related, do you think you could create a test showing the problem so that a patch can be accepted ?

  • First commit to issue fork.
  • Merge request !11493Added a unit test β†’ (Open) created by dcam
  • Pipeline finished with Failed
    about 1 month ago
    Total: 298s
    #449403
  • Pipeline finished with Success
    about 1 month ago
    Total: 822s
    #449408
  • πŸ‡ΊπŸ‡ΈUnited States dcam

    I converted the patch in #3 to an MR and added a unit test.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Still have a hard time seeing the use case of setting the title to NULL but does have test coverage so lets see what the committers think.

  • πŸ‡³πŸ‡ΏNew Zealand quietone
Production build 0.71.5 2024