Fixing JsonPatchDocument bug (#65470)

src/Features/JsonPatch.SystemTextJson/src/Internal/ListAdapter.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Text.Json;
 using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Helpers;
@@ -147,11 +146,9 @@ internal class ListAdapter : IAdapter
 
     public virtual bool TryTraverse(object target, string segment, JsonSerializerOptions serializerOptions, out object value, out string errorMessage)
     {
-        var list = target as IList;
-        if (list == null)
+        if (!TryGetListTypeArgument(target, out _, out errorMessage))
         {
             value = null;
-            errorMessage = null;
             return false;
         }
 
@@ -162,14 +159,16 @@ internal class ListAdapter : IAdapter
             return false;
         }
 
-        if (index < 0 || index >= list.Count)
+        var count = GenericListOrJsonArrayUtilities.GetCount(target);
+
+        if (index < 0 || index >= count)
         {
             value = null;
             errorMessage = Resources.FormatIndexOutOfBounds(segment);
             return false;
         }
 
-        value = list[index];
+        value = GenericListOrJsonArrayUtilities.GetElementAt(target, index);
         errorMessage = null;
         return true;
     }

src/Features/JsonPatch.SystemTextJson/test/IntegrationTests/AnonymousObjectIntegrationTest.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Text.Json.Nodes;
 using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Exceptions;
 using Xunit;
 
@@ -162,4 +163,44 @@ public class AnonymousObjectIntegrationTest
         Assert.Equal("The current value 'A' at path 'StringProperty' is not equal to the test value 'B'.",
             exception.Message);
     }
+
+    [Fact]
+    public void DeeplyNestedArrayTraversal_ReplaceSucceeds()
+    {
+        //Arrange
+        var doc = new JsonObject
+        {
+            ["data"] = new JsonObject
+            {
+                ["levels"] = new JsonArray
+                {
+                    null,
+                    new JsonObject
+                    {
+                        ["items"] = new JsonArray
+                        {
+                            new JsonObject { ["id"] = 100 },
+                            new JsonObject
+                            {
+                                ["details"] = new JsonArray
+                                {
+                                    new JsonObject { ["value"] = "old" },
+                                    new JsonObject { ["value"] = "target" },
+                                    new JsonObject { ["value"] = 999 }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        };
+
+        var patch = new JsonPatchDocument().Replace("/data/levels/1/items/1/details/1/value", "updated");
+
+        //Act
+        patch.ApplyTo(doc);
+
+        //Assert
+        Assert.Equal("updated", doc["data"]["levels"][1]["items"][1]["details"][1]["value"].GetValue<string>());
+    }
 }

src/Features/JsonPatch.SystemTextJson/test/Internal/ListAdapterTest.cs

@@ -5,6 +5,7 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Text.Json;
+using System.Text.Json.Nodes;
 using Xunit;
 
 namespace Microsoft.AspNetCore.JsonPatch.SystemTextJson.Internal;
@@ -484,7 +485,7 @@ public class ListAdapterTest
     {
         // Arrange
         var serializerOptions = JsonSerializerOptions.Default;
-        var targetObject = new List<int>() { 10, 20 };
+        var targetObject = new JsonArray { 10, 20 };
         var listAdapter = new ListAdapter();
         var expectedErrorMessage = "The index value provided by path segment '2' is out of bounds of the array size.";
 
@@ -495,4 +496,116 @@ public class ListAdapterTest
         Assert.False(testStatus);
         Assert.Equal(expectedErrorMessage, errorMessage);
     }
+
+    [Theory]
+    [InlineData(new int[] { }, "0")]
+    [InlineData(new int[] { 10, 20 }, "-1")]
+    [InlineData(new int[] { 10, 20 }, "2")]
+    public void TryTraverse_IndexOutOfBounds_ReturnsFalse(int[] input, string segment)
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new List<int>(input);
+        var listAdapter = new ListAdapter();
+
+        // Act
+        var success = listAdapter.TryTraverse(targetObject, segment, serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.False(success);
+        Assert.Null(value);
+        Assert.Equal($"The index value provided by path segment '{segment}' is out of bounds of the array size.", message);
+    }
+
+    [Fact]
+    public void TryTraverse_JsonArray_ValidNumericIndex_ReturnsElement()
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new JsonArray() {10, 20, 30};
+        var expectedValue = 20;
+        var segment = "1";
+        var listAdapter = new ListAdapter();
+
+        // Act
+        var success = listAdapter.TryTraverse(targetObject, segment, serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.True(success);
+        var jsonValue = Assert.IsAssignableFrom<JsonValue>(value);
+        Assert.Equal(expectedValue, jsonValue.GetValue<int>());
+        Assert.Null(message);
+    }
+
+    [Theory]
+    [InlineData("-")]
+    [InlineData("abc")]
+    [InlineData("3.14")]
+    [InlineData("")]
+    public void TryTraverse_InvalidSegmentFormat_ReturnsFalse(string segment)
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new List<string> { "a", "b", "c" };
+        var listAdapter = new ListAdapter();
+
+        // Act
+        var success = listAdapter.TryTraverse(targetObject, segment, serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.False(success);
+        Assert.Null(value);
+        Assert.Equal($"The path segment '{segment}' is invalid for an array index.", message);
+    }
+
+    [Fact]
+    public void TryTraverse_JsonArray_OnEmptyList_WithValidIndex_ReturnsFalse()
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new JsonArray();
+        var listAdapter = new ListAdapter();
+        var segment = "0";
+        // Act
+        var success = listAdapter.TryTraverse( targetObject, segment, serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.False(success);
+        Assert.Null(value);
+        Assert.Equal($"The index value provided by path segment '{segment}' is out of bounds of the array size.", message);
+    }
+
+    [Fact]
+    public void TryTraverse_NonGenericList_Fails()
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new ArrayList { 10, 20, 30 };
+        var listAdapter = new ListAdapter();
+
+        // Act
+        var success = listAdapter.TryTraverse(targetObject, "1", serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.False(success);
+        Assert.Null(value);
+        Assert.Equal($"The type '{targetObject.GetType().FullName}' which is a non generic list is not supported for json patch operations. Only generic list types are supported.", message);
+    }
+
+    [Fact]
+    public void TryTraverse_Array_Fails()
+    {
+        // Arrange
+        var serializerOptions = JsonSerializerOptions.Default;
+        var targetObject = new[] { 100, 200, 300 };
+        var listAdapter = new ListAdapter();
+
+        // Act
+        var success = listAdapter.TryTraverse(targetObject, "0", serializerOptions, out var value, out var message);
+
+        // Assert
+        Assert.False(success);
+        Assert.Null(value);
+        Assert.Equal($"The type '{targetObject.GetType().FullName}' which is an array is not supported for json patch operations as it has a fixed size.", message);
+    }
 }