Skip to content

Media: Add support for Media URL providers with cache busting (Closes #23282) - #23327

Merged
Zeegaan merged 3 commits into
v17/devfrom
v17/bugfix/support-media-url-provider-with-cache-busting
Jul 12, 2026
Merged

Media: Add support for Media URL providers with cache busting (Closes #23282)#23327
Zeegaan merged 3 commits into
v17/devfrom
v17/bugfix/support-media-url-provider-with-cache-busting

Conversation

@kjac

@kjac kjac commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #23282 (in part)

Description

Umbraco does not automatically append cache busting parameters to raw media URLs. This is by design, to prevent cache misses downstream.

This does however come with a few drawbacks, as described in ##23282.

One would then assume that a custom Media URL provider would be able to solve these drawbacks for a concrete implementation --- but unfortunately, the rich text rendering assumes a bit too much when rendering local image sources; it does not expect Media URLs with query string parameters.

In effect, if a Media URL provider yields Media URL with a cache buster parameter (e.g. /media/something/image.png?rand=123), the rich text conversion ends up generating Media URLs with two ? parts (/media/something/image.png?rand=123?rmode=max&....).

This PR ensures that the rich text rendering appends query string parameters safely, thus enabling Media URL providers with cache busting by query string.

Note

Crop URLs are cache busted by default as-is, because the effective crops are expected to change over time.

Testing this PR

Replace the default Media URL provider with a custom implementation which adds cache busting (example code below), and verify that:

  1. Rich text rendering produces valid Media URLs which include the cache busting part.
  2. Media picker "bare" URLs contain the cache busting part.
  3. Media picker crop URLs also contain the cache busting part (despite them already having a cache buster by default).

Verify all of the above both for templated rendering (example template below) and for the Delivery API output.

Example template

@using Umbraco.Cms.Core.Models
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage
@{
	Layout = null;
    var cropUrl = Model.Value<MediaWithCrops>("mediaPicker")?.GetCropUrl("small");
    var imageUrl = Model.Value<MediaWithCrops>("mediaPicker")?.Url();
}
<html>
<body>
<h2>Bare Media URL</h2>
<img src="@imageUrl" width="300px"/>
<hr/>
<h2>Media crop URL</h2>
<img src="@cropUrl"/>
<hr/>
<h2>Rich text</h2>
@Model.Value("richtextEditor")
</body>
</html>

Example Media URL provider

using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Routing;

namespace My.Site.Providers;

public class MyMediaUrlProvider : DefaultMediaUrlProvider
{
    public MyMediaUrlProvider(MediaUrlGeneratorCollection mediaPathGenerators, IUrlAssembler urlAssembler)
        : base(mediaPathGenerators, urlAssembler)
    {
    }

    public override UrlInfo? GetMediaUrl(IPublishedContent content, string propertyAlias, UrlMode mode, string? culture, Uri current)
    {
        UrlInfo? urlInfo = base.GetMediaUrl(content, propertyAlias, mode, culture, current);
        if (urlInfo?.Url is null)
        {
            return urlInfo;
        }

        // assemble a new Media URL with a calculated cache buster
        var cacheBuster = content.UpdateDate.Ticks;
        var url = new Uri($"{urlInfo.Url}?r={cacheBuster}", urlInfo.Url.IsAbsoluteUri ? UriKind.Absolute : UriKind.Relative);
        return new UrlInfo(url, Constants.UrlProviders.Media, culture);
    }
}

public class MyMediaUrlProviderComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
        => builder.MediaUrlProviders().Replace<DefaultMediaUrlProvider, MyMediaUrlProvider>();
}

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @kjac's task in 6m 4s —— View job


PR Review

Target: origin/v17/dev · Based on commit: 324eeaf1 · Skipped: 0 noise files

Fixes the double-? bug that occurred when a custom Media URL provider appended cache-busting query parameters and rich text rendering naively concatenated the stored query string onto the already-parameterised URL. The core change — swapping $"{mediaUrl}{queryString}" for mediaUrl.AppendQueryStringToUrl(queryString) — is exactly right for the happy path and is covered by new tests.

  • Other changes: Rich text rendering now always calls AppendQueryStringToUrl instead of string concatenation; behavior changes for images with cache-busting URLs (intended) and, inadvertently, for images without any query string (see Important findings below).

Important

  • src/Umbraco.Core/Templates/HtmlImageSourceParser.cs:139: When the stored image src has no query string (plain, uncropped image), src.Groups[2].Value is "". AppendQueryStringToUrl("") filters the empty string out of nonEmpty, then calls "".EnsureStartsWith('?') = "?" (or '&'), appending a stray trailing character to the URL — a regression vs. the old $"{mediaUrl}{""}" which cleanly produced mediaUrl. Guard before delegating: queryString.IsNullOrWhiteSpace() ? mediaUrl : mediaUrl.AppendQueryStringToUrl(queryString).

  • src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs:129: Same issue — currentImageQueryString is now string.Empty (previously null) when no ? is present. null was safe in the old string interpolation; "" is not safe as an argument to AppendQueryStringToUrl. Same guard pattern fixes both files.

Suggestions

  • tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlImageSourceParserTests.cs:281: The new tests cover the with-query-string path. Adding a test for a plain image (no crop params in src) + cache-busting provider would prevent the regression from going undetected. The suggested test is on the inline comment.

Request Changes

The fix correctly addresses the primary bug, but the empty-query-string edge case introduces a regression (trailing ? or & on plain image URLs) that should be fixed before merging.

@kjac kjac changed the title Add support for Media URL providers with cache busting Media: Add support for Media URL providers with cache busting (Closes #23282) Jul 8, 2026
Comment thread src/Umbraco.Core/Templates/HtmlImageSourceParser.cs Outdated
Comment thread src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs Outdated
@sonarqubecloud

Copy link
Copy Markdown

@Zeegaan Zeegaan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works like a charm 🚀

@Zeegaan
Zeegaan merged commit 168f9af into v17/dev Jul 12, 2026
30 of 31 checks passed
@Zeegaan
Zeegaan deleted the v17/bugfix/support-media-url-provider-with-cache-busting branch July 12, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants