Permissions: Protect GetIdsFromPathReversed against invalid program exception

[AndyButland]

Description

This came from a report on an upgraded site on Umbraco Cloud:

After the upgrade my content is no longer visible in the content tree and when visiting the backoffice I see the error message: "An error occurred. Cannot create boxed ByRef-like values."
This same error message also appears if I try to create a new content item in the tree and save and publish.

Stack trace:

System.InvalidProgramException: Cannot create boxed ByRef-like values.
   at Umbraco.Extensions.StringExtensions.GetIdsFromPathReversed(String path)
   at Umbraco.Extensions.StringExtensions.GetIdsFromPathReversed(String path)
   at Umbraco.Cms.Core.Services.UserService.GetPermissionsForPath(IUser user, String path)
   at Umbraco.Cms.Core.Services.ContentPermissionService.HasPermissionAccess(IUser user, IEnumerable`1 contentPaths, ISet`1 permissionsToCheck)
   at Umbraco.Cms.Core.Services.ContentPermissionService.AuthorizeRootAccessAsync(IUser user, ISet`1 permissionsToCheck)
   at Umbraco.Cms.Core.Security.Authorization.ContentPermissionAuthorizer.IsDeniedAtRootLevelAsync(IUser currentUser, ISet`1 permissionsToCheck)
   at Umbraco.Cms.Api.Management.Security.Authorization.Content.ContentPermissionHandler.IsAuthorized(AuthorizationHandlerContext context, ContentPermissionRequirement requirement, ContentPermissionResource resource)
   at Umbraco.Cms.Api.Management.Security.Authorization.MustSatisfyRequirementAuthorizationHandler`2.HandleRequirementAsync(AuthorizationHandlerContext context, T requirement, TResource resource)
   at Microsoft.AspNetCore.Authorization.AuthorizationHandler`2.HandleAsync(AuthorizationHandlerContext context)
   at Microsoft.AspNetCore.Authorization.DefaultAuthorizationService.AuthorizeAsync(ClaimsPrincipal user, Object resource, IEnumerable`1 requirements)
   at Microsoft.AspNetCore.Authorization.DefaultAuthorizationServiceImpl.AuthorizeAsync(ClaimsPrincipal user, Object resource, String policyName)
   at Umbraco.Cms.Api.Management.Controllers.Document.CreateDocumentControllerBase.HandleRequest(CreateDocumentRequestModel requestModel, Func`1 authorizedHandler)
   at Umbraco.Cms.Api.Management.Controllers.Document.CreateDocumentController.Create(CancellationToken cancellationToken, CreateDocumentRequestModel requestModel)

I couldn't replicate this via unit tests, but from some AI help got the advice to change as indicated by the comment I've added to the code.

// Using the explicit enumerator (while/MoveNext) over the SpanSplitEnumerator in a forach loop to avoid any compiler
// boxing of the ref struct enumerator.
// This prevents potential InvalidProgramException across compilers/JITs ("Cannot create boxed ByRef-like values.").

Why couldn't a unit test be created to replicate this?

Because on your current compiler/runtime the span-based enumerator is handled without boxing, the test will not throw. The original InvalidProgramException: Cannot create boxed ByRef-like values happens only when the compiler/JIT emits code that tries to box a byref-like (ref struct) enumerator (e.g. treating it as an IDisposable/interface or capturing it on the heap). Modern Roslyn/.NET emit code that avoids that boxing for ReadOnlySpan.Split() so you won't hit the exception on current toolchain.

If you want to hard-reproduce the legacy bug you need an older compiler/JIT combination that produces the bad IL (or a specifically contrived boxing path) — not something you can reproduce reliably on .NET 10 / current Roslyn.

So why this happened on this Cloud site I'm not clear on, but nonetheless this update continues to pass the existing unit tests, plus some additions, so I think it's a good, defensive update to apply.

[Copilot]

Pull request overview

This PR addresses an InvalidProgramException reported on Umbraco Cloud after an upgrade, caused by boxing of a ref struct enumerator. The fix refactors GetIdsFromPathReversed to use an explicit enumerator pattern instead of foreach to prevent potential boxing issues across different compilers/JITs. The PR also adds test coverage for null and empty string inputs and improves the XML documentation.

Key Changes:

• Refactored GetIdsFromPathReversed from foreach loop to explicit while/MoveNext pattern to avoid ref struct boxing
• Added test cases for null and empty string inputs
• Enhanced XML documentation with parameter and return value descriptions

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/Umbraco.Core/Extensions/StringExtensions.cs	Refactored GetIdsFromPathReversed to use explicit enumerator pattern instead of foreach, added defensive comment explaining the change, improved XML documentation
tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs	Added null and empty string test cases, renamed test method for clarity

[Copilot]

Calling AsSpan() on a null string will throw a NullReferenceException. The test cases at lines 392-393 test null and empty strings, so this method needs to handle null input. Consider adding a null check at the beginning: if (path is null or "") return Array.Empty<int>(); or similar.

Suggested change

	{
	{
	if (string.IsNullOrEmpty(path))
	{
	return Array.Empty<int>();
	}

[AndyButland]

This this is OK as is. We accept string, not string? as input to this function. I added a null input for the test just to verify that we don't get a failure if somehow a null was coerced in.