Skip to content

Permissions: Protect GetIdsFromPathReversed against invalid program exception #20930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+14 −5
Open

Permissions: Protect GetIdsFromPathReversed against invalid program exception #20930

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7053df3
Updates to protect GetIdsFromPathReversed against invalid program exc…
AndyButland Nov 22, 2025
baf4983
Apply suggestions from code review
AndyButland Nov 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/Umbraco.Core/Extensions/StringExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,23 @@ static StringExtensions()
}

/// <summary>
/// Convert a path to node ids in the order from right to left (deepest to shallowest)
/// Convert a path to node ids in the order from right to left (deepest to shallowest).
/// </summary>
/// <param name="path"></param>
/// <returns></returns>
/// <param name="path">The path string expected as a comma delimited collection if integers.</param>
/// <returns>An array of integers matching the provided path.</returns>
public static int[] GetIdsFromPathReversed(this string path)
{
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

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

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>();
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@AndyButland AndyButland Nov 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

ReadOnlySpan<char> pathSpan = path.AsSpan();

// 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.").
MemoryExtensions.SpanSplitEnumerator<char> pathSegmentsEnumerator = pathSpan.Split(Constants.CharArrays.Comma);

List<int> nodeIds = [];
foreach (Range rangeOfPathSegment in pathSpan.Split(Constants.CharArrays.Comma))
while (pathSegmentsEnumerator.MoveNext())
{
Range rangeOfPathSegment = pathSegmentsEnumerator.Current;
if (int.TryParse(pathSpan[rangeOfPathSegment], NumberStyles.Integer, CultureInfo.InvariantCulture, out int pathSegment))
{
nodeIds.Add(pathSegment);
Expand Down
4 changes: 3 additions & 1 deletion tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,11 @@ private static void TryIsFullPath(string path, bool expectedIsFull, bool expecte
}
}

[TestCase(null, "")]
[TestCase("", "")]
[TestCase("1,2,3,4,5", "5,4,3,2,1")]
[TestCase("1,2,x,4,5", "5,4,2,1")]
public void GetIdsFromPathReversed(string input, string expected)
public void GetIdsFromPathReversed_ReturnsExpectedResult(string input, string expected)
{
var ids = input.GetIdsFromPathReversed();
Assert.AreEqual(expected, string.Join(",", ids));
Expand Down
Loading