Problem/Motivation

\Drupal\Core\Controller\TitleResolver::getTitle() adds all the raw parameters of a route to the placeholder arguments array by doing:

      if (($raw_parameters = $request->attributes->get('_raw_variables'))) {
        foreach ($raw_parameters->all() as $key => $value) {
          $args['@' . $key] = $value;
          $args['%' . $key] = $value;
        }
      }

If a raw parameter has a NULL value this will be added to the $args array. In PHP 8.1 this will trigger deprecation notices because we'll end up passing these NULL values to a built-in function that expects a string - namely htmlspecialchars().

Steps to reproduce

Proposed resolution

This deprecation should be fixed where the values are assigned - ie. \Drupal\Core\Controller\TitleResolver::getTitle(). It is tempting to fix in \Drupal\Component\Utility\Html::escape(), \Drupal\Component\Render\FormattableMarkup::placeholderEscape() or \Drupal\Component\Render\FormattableMarkup::placeholderFormat() - however these method lack the context to determine where a NULL value is an unexpected error and we should work why there is no string or it is safe to replace with an empty string.

Additionally fixing it this will allow us to add typehints to \Drupal\Component\Render\FormattableMarkup::placeholderEscape() and \Drupal\Component\Utility\Html::escape() to improve the robustness of our code.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

CommentFileSizeAuthor
#2 3236789-2.patch2.01 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
2.01 KB

Here's the change from #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves) plus an additional test to ensure that TitleResolver behaves as we expect it to.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Rationale makes sense, definitely agree on fixing these at source rather than patching where the error occurs. Code looks good to me, RTBC if bot agrees.

  • larowlan committed 76b1a8b on 9.3.x
    Issue #3236789 by alexpott: Drupal\Core\Controller\TitleResolver::...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed 76b1a8b and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

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