🇺🇸United States @totten

Added forward ports for 10.1.x and 11.x. Tests still passing.

Added a forward-port (from !3953 on 9.5.x to !4129 on 10.0.x). The merge-conflict was fairly small. The new test passed locally, and I confirmed that one specific error scenario ( re: civicrm-core@5.60 ✨ Add Exception for TypeError Argument must be String in \Drupal\Component\Utility\Html escape{} Needs work ) worked with this fix.

Aside: This is the first time I've posted a patch to drupal core, so I'm not too familiar with the workflows. But I can say that the merge-conflict should be resolved -- you can cherry-pick the 10.0.x patch on 10.1.x or 11.x.

Yeah, that `Route` metadata seems to go through several copy/filter steps, and I don't know the history on all of them, but you only need one of them to be more selective.

TitleResolver should check for route parameters being a scalar before copying them as translation arguments:

+1 - Sounds sensible to me. That seems like the place where you go from a broader domain (all possible arguments to a page-controller) to a narrower domain (values that can be interpolated into a string).

It's possible that the "Breadcrumb Scenario (CiviCRM)" is a little different from the original filing on this issue. In particular, the initial description talks of a problem experienced in 9.5.7 with cron jobs. For the Breadcrumb/CiviCRM scenario, 9.5.7 was OK -- problem begain in 9.5.9.

Related discussion: https://www.drupal.org/project/drupal/issues/3277784 🐛 copyRawVariables should support default route parameters Fixed

Over at CiviCRM.org, we have an E2E test-suite for checking the integration between Civi and D9/D7/BD/et al. Following yesterday's release, some of the D9-Civi tests have been raising a red-flag for this issue. I can give steps-to-reproduce, but they are specific to Civi, so it's probably more useful to describe the situation.

Breadcrumb Scenario - General description

1. The scenario involves a page-request for /civicrm/dev/fake-error. (Note: Routes are mapped between Civi and D9 via Drupal\civicrm\Routing\Routes.)
2. The page request fails because "TypeError: htmlspecialchars(): Argument #1 ($string) must be of type string, array given".
3. Specifically, while rendering this page, D9 attempts to show breadcrumbs ("olivero_breadcrumbs"). Each breadcrumb has a title (instance of "TranslateableMarkup"").
4. In some breadcrumb titles, the "TranslateableMarkup::$arguments" include array values.
5. Breadcrumb titles are generated by "TitleResolver", which copies all data from $request->attributes->get("_raw_variables") to t(...$args...).

This seems to be the conflict:

1. TitleResolver is trying to copy every variable that might potentially be useful for the title-string. (It doesn't seem to care which variables are actually used.)
2. Html::escape is trying to escape every variable that might potentially be used in the title. (It doesn't seem to care which variables are actually used.)
3. There are some request-attributes (like "args") which are not intended for page-titles, and they are referenced by page-titles, and are not simple strings.

In short, invisible metadata is getting copied-around and unnecessarily subjected to escaping-rules that it doesn't pass and doesn't need to pass.

Perhaps the simplest change would be to update FormattableMarkup::placeholderFormat() to apply escaping lazily? e.g. use preg_replace_callback() instead of strtr()?

Breadcrumb Scenario - Specifics

I observed this problem while debugging a failure in CiviCRM's E2E\Core\ErrorTest.

The purpose of the test is to provoke errors and confirm that the Civi-Drupal integration displays those errors in a semi-sensible way. If you have an environment that's configured for running this test-suite, then the way to execute is:

cd vendor/civicrm/civicrm-core
phpunit8 --debug tests/phpunit/E2E/Core/ErrorTest.php

(I'm not sure if this specific test is easy to run from a Drupal POV. Many Civi tests require some setup. OTOH, E2E tests don't require as much setup. If it's necessary/helpful, I can come up with more pointers for running it or provide a temporary VM for running it.)