ImageUrlFormatter HTML-escapes ampersands in image style URLs when a second query parameter is present

Problem/Motivation

Identified originally while using 'URL to Image' formatter in a REST Export View, with a rewrite rule like {{ url('<front>') }}{{ field_media_image|trim('/') }}. The view preview shows some of the double encoding /sites/default/files/styles/responsive_1_1_800w/..../image.jpg.webp?h=6b1e78c7&itok=AJZNiD9O. The REST Export endpoint shows/sites/default/files/styles/responsive_1_1_800w/..../image.jpg.webp?h=6b1e78c7&itok=AJZNiD9O. With ..&itok.. in URL a consumer of the REST Endpoint can try that exact URL, if a style derivative is already available, this works fine. When a new style derivative needs created it fails. Related issues with workarounds were found in various contrib modules, but the better solve might be this tweak to the core formatter.

Found on Drupal 11.3, where the tests below were run. The relevant code is unchanged on main, the ['#markup' => $url] assignment in ImageUrlFormatter::viewElements(), Renderer::ensureMarkupIsSafe(), and the ampersand substitution in Xss::filter() are all identical there. The MR targets main.

ImageUrlFormatter returns the URL as a plain string in #markup. Renderer::ensureMarkupIsSafe() XSS-filters any #markup that isn't a MarkupInterface, and the first thing Xss::filter() does is replace every & with &. It restores real entities afterwards, but &itok= isn't one, so it stays escaped.

Core normally puts only one query parameter on an image style URL, so there's no ampersand and nothing breaks. Add a module that appends a second one and the URL is corrupted. Crop does this (?h=).

This is harmless in HTML, & is correct there and browsers request the right URL. It matters because this formatter exists for REST exports #2517030: Add a URL formatter for the image field. In a serialized response the escaped string is passed through verbatim, so the consumer gets a parameter named amp;itok and no itok at all. ImageStyleDownloadController then 404s any derivative that isn't already on disk. Derivatives that do exist are served by the webserver and never reach the controller, so only some images break and the pattern looks random.

Steps to reproduce

1. Image field on Article, any image style, formatter set to URL to image.
2. Enable a module appending a second query parameter to derivative URLs. Or something like Focal Point Crop
   

   use Drupal\Core\StreamWrapper\StreamWrapperManager;
   
   /**
    * Implements hook_file_url_alter().
    */
   function example_file_url_alter(&$uri) {
     // Only act on image style derivatives.
     if (!str_contains($uri, '/styles/')) {
       return;
     }
   
     // Resolve to an external URL first. Appending to the stream URI instead
     // would send the query separator through UrlHelper::encodePath(), which
     // turns "?" into "%3F". This mirrors crop_file_url_alter().
     $scheme = StreamWrapperManager::getScheme($uri);
     if ($scheme && !in_array($scheme, ['http', 'https', 'data'])) {
       if ($wrapper = \Drupal::service('stream_wrapper_manager')->getViaUri($uri)) {
         $uri = $wrapper->getExternalUrl();
       }
     }
   
     $uri .= (str_contains($uri, '?') ? '&' : '?') . 'h=abc12345';
   }
   

3. Render the field and look at the raw string, viewing the page won't show it. Try using a REST View Export or just Drush:
   

   drush php:eval '
   $node = \Drupal\node\Entity\Node::load(1);
   $build = $node->get("field_image")->view([
     "type" => "image_url",
     "settings" => ["image_style" => "wide"],
   ]);
   print (string) \Drupal::service("renderer")->renderInIsolation($build) . "\n";
   '
   

Actual ?h=abc12345&itok=... Expected ?h=abc12345&itok=...

Proposed resolution

Return Markup::create($url) so ensureMarkupIsSafe() leaves it alone.

#allowed_tags doesn't help, the ampersand substitution happens before any tag handling. #plain_text doesn't either, it routes through Html::escape().

Marking it safe doesn't widen the attack surface. The filename is already percent-encoded by UrlHelper::encodePath() (rawurlencode, %2F restored) before the formatter sees it, so <, >, " and ' can't survive in the path segment. I can post the full trace if that's worth spelling out.

Fixing this downstream in rest/serialization isn't an option. The value is already escaped by the time DataFieldRow reads last_render. The formatter is where the string is produced, so it's where it has to be marked safe.

The change

Two lines in ImageUrlFormatter — a use Drupal\Core\Render\Markup; import and:

-      $elements[$delta] = ['#markup' => $url];
+      $elements[$delta] = ['#markup' => Markup::create($url)];

Test coverage

No new test module. file_test already has a state-driven hook_file_url_alter() with cdn, root-relative and protocol-relative modes, and its comment says it works that way to avoid another hidden test module. The MR adds a fourth mode, query-parameter, that appends a second parameter to derivative URLs the way crop does.

ImageUrlFormatterTest (new, kernel) covers both cases: a derivative URL with two query parameters, asserting the separator is not escaped, and one with only itok, asserting normal output is unchanged.

Verified locally on 11.3 before opening the MR. Against unpatched core, testAmpersandBetweenQueryParametersIsNotEscaped fails on:

Failed asserting that '…/test-image.png.avif?h=abc12345&itok=mfFLxFb9'
does not contain "&".

With the two-line change applied, both tests pass.

Duplicates

I couldn't find a duplicate in core. Closest is #3276933: Responsive image formatter url encoding (responsive image formatter, different symptom). #2273925: Ensure #markup is XSS escaped in Renderer::doRender() introduced the #markup filtering this relies on.

Six contrib projects have worked around the same behaviour independently: rest_views #3176391: Image links with multiple query parameters have ampersand encoded, image_url_formatter #3225415: URL formatter html-encodes ampersands in URLs , svg_image #3337927: Image links with multiple query parameters have ampersand encoded in REST view, metatag #2962426: Image URL double encoded, token #3137689: Image URL tokens are HTML escaped, bg_image_formatter #2937226: Image styles won't generate because of auto-escaped image URLs.

Remaining tasks

1. Review MR
2. Backport

User interface changes

None.

API changes

#markup becomes a MarkupInterface rather than a string. Stringable, so existing consumers are unaffected.

Data model changes

None.

Release notes snippet

AI-Generated: Yes (Used Claude to help debugging, generated the research, test building, identifying related issues and URL sanitization testing.