Throw if attempting to use the default unique media path scheme with version 7 GUIDs (#19419)

* Thow if attempting to use the default unique media path scheme with version 7 GUIDs.

* Expanded unittests, fixed null params, chose a better exception

* Use parameters in test.

---------

Co-authored-by: Migaroez <[email protected]>

Author: AndyButland

src/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathScheme.cs

@@ -13,6 +13,18 @@ public class UniqueMediaPathScheme : IMediaPathScheme
     /// <inheritdoc />
     public string GetFilePath(MediaFileManager fileManager, Guid itemGuid, Guid propertyGuid, string filename)
     {
+        // Shortening of the Guid to 8 chars risks collisions with GUIDs, which are expected to be very rare.
+        // However with GUID 7, the chance is much higher, as those created in a short period of time could have
+        // the same first 8 characters.
+        // Such GUIDs would have been created via Guid.CreateVersion7() rather than Guid.NewGuid().
+        // We should detect that, throw, and recommend creation of standard GUIDs or the use of a custom IMediaPathScheme instead.
+        if (itemGuid.Version == 7 || propertyGuid.Version == 7)
+        {
+            throw new ArgumentException(
+                "The UniqueMediaPathScheme cannot be used with version 7 GUIDs due to an increased risk of collisions in the generated file paths. " +
+                "Please use version 4 GUIDs created via Guid.NewGuid() or implement and register a different IMediaPathScheme.");
+        }
+
         Guid combinedGuid = GuidUtils.Combine(itemGuid, propertyGuid);
         var directory = GuidUtils.ToBase32String(combinedGuid, DirectoryLength);
 

tests/Umbraco.Tests.UnitTests/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathSchemeTests.cs

@@ -0,0 +1,43 @@
+using Microsoft.Extensions.Logging;
+using Moq;
+using NUnit.Framework;
+using Umbraco.Cms.Core.IO;
+using Umbraco.Cms.Core.IO.MediaPathSchemes;
+using Umbraco.Cms.Core.Strings;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.IO.MediaPathSchemes;
+
+[TestFixture]
+public class UniqueMediaPathSchemeTests
+{
+    private static MediaFileManager MediaFileManager => new(
+        Mock.Of<IFileSystem>(),
+        Mock.Of<IMediaPathScheme>(),
+        Mock.Of<ILogger<MediaFileManager>>(),
+        Mock.Of<IShortStringHelper>(),
+        Mock.Of<IServiceProvider>());
+
+    [Test]
+    public void GetFilePath_Creates_ExpectedPath()
+    {
+        var scheme = new UniqueMediaPathScheme();
+        var itemGuid = new Guid("00000000-0000-4000-0000-000000000001");
+        var propertyGuid = new Guid("00000000-0000-4000-0000-000000000002");
+        var filename = "test.txt";
+        string actualPath = scheme.GetFilePath(MediaFileManager, itemGuid, propertyGuid, filename);
+        Assert.AreEqual("aaaaaaaa/test.txt", actualPath);
+    }
+
+    [TestCase(true, false)]
+    [TestCase(false, true)]
+    [TestCase(true, true)]
+    public void GetFilePath_ShouldThrow_WhenUsingVersion7Guids(bool userVersion7ForItemGuid, bool userVersion7ForPropertyGuid)
+    {
+        var scheme = new UniqueMediaPathScheme();
+        var itemGuid = new Guid($"00000000-0000-{(userVersion7ForItemGuid ? "7" : "4")}000-0000-000000000001");
+        var propertyGuid = new Guid($"00000000-0000-{(userVersion7ForPropertyGuid ? "7" : "4")}000-0000-000000000002");
+        var filename = "test.txt";
+
+        Assert.Throws<ArgumentException>(() => scheme.GetFilePath(MediaFileManager, itemGuid, propertyGuid, filename));
+    }
+}