Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ public override void WriteJson(
writer.WritePropertyName(PatchConstants.PropertyNames.OperationType);
writer.WriteValue(operation.OperationType.ToEnumMemberString());
writer.WritePropertyName(PatchConstants.PropertyNames.Path);
writer.WriteValue(operation.Path);
writer.WriteValue(PatchPathHelper.ProcessPath(operation.Path));

if (operation.OperationType == PatchOperationType.Move)
{
writer.WritePropertyName(PatchConstants.PropertyNames.From);
writer.WriteValue(operation.From);
writer.WriteValue(PatchPathHelper.ProcessPath(operation.From));
}
else if (operation.TrySerializeValueParameter(this.userSerializer, out Stream valueStream))
{
Expand Down
99 changes: 99 additions & 0 deletions Microsoft.Azure.Cosmos/src/Patch/PatchPathHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
//------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
//------------------------------------------------------------

namespace Microsoft.Azure.Cosmos
{
using System;
using System.Globalization;
using System.Text;
using System.Text.RegularExpressions;

/// <summary>
/// Helper class for processing JSON patch paths to handle numeric-looking strings
/// </summary>
internal static class PatchPathHelper
{
private const int MaxNumericTokenLength = 19;
private static readonly Regex NumericRegex = new Regex(@"^\d+$", RegexOptions.Compiled);

/// <summary>
/// Processes a path to ensure long numeric-looking strings are handled correctly
/// </summary>
/// <param name="path">The original path</param>
/// <returns>The processed path with proper escaping for long numeric strings</returns>
public static string ProcessPath(string path)
{
if (string.IsNullOrEmpty(path))
{
return path;
}

// Split the path into segments
var segments = path.Split('/');
var result = new StringBuilder();
bool isFirstSegment = true;

for (int i = 0; i < segments.Length; i++)
{
var segment = segments[i];

// First segment is empty for paths starting with '/'
if (i == 0 && string.IsNullOrEmpty(segment))
{
result.Append('/');
continue;
}

if (!isFirstSegment)
{
result.Append('/');
}

// Check if segment is a long numeric string
if (IsLongNumericString(segment))
{
// Escape by prefixing with a zero-width character approach
// or by using the array index notation to force string interpretation
result.Append(EscapeNumericString(segment));
}
else
{
result.Append(segment);
}

isFirstSegment = false;
}

return result.ToString();
}

/// <summary>
/// Checks if a string is a numeric string longer than the maximum allowed length
/// </summary>
/// <param name="segment">The path segment to check</param>
/// <returns>True if it's a long numeric string that needs escaping</returns>
private static bool IsLongNumericString(string segment)
{
if (string.IsNullOrEmpty(segment))
{
return false;
}

// Check if it's numeric and longer than the maximum allowed length
return NumericRegex.IsMatch(segment) && segment.Length > MaxNumericTokenLength;
}

/// <summary>
/// Escapes a numeric string to ensure it's treated as a property name
/// </summary>
/// <param name="numericString">The numeric string to escape</param>
/// <returns>The escaped string</returns>
private static string EscapeNumericString(string numericString)
{
// Use array index notation to force string interpretation
// This tells the server to treat it as a string property rather than a numeric index
return $"[\"{numericString}\"]";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4081,9 +4081,130 @@ public async Task DeleteItemAsyncTest(bool binaryEncodingEnabled)
}
}

[TestMethod]
[DataRow(false, DisplayName = "QueryItemAsyncTest - Binary encoding disabled.")]
[DataRow(true, DisplayName = "QueryItemAsyncTest - Binary encoding enabled.")]
[TestMethod]
public async Task ItemPatchWithLongNumericStringPathTest()
{
// This test mirrors the original issue scenario from #5154
// The issue was that patch operations would fail with "Token('12345678901234567890') has length longer than expected"
// when paths contained numeric-looking strings 20+ characters long

// Create a test document with a flexible structure to hold dynamic properties
var testDocument = new
{
id = Guid.NewGuid().ToString(),
pk = "test-partition-key",
strings = new Dictionary<string, object>(),
description = "Test document for long numeric string paths"
};

// Create the document in the container
await this.Container.CreateItemAsync(testDocument, new Cosmos.PartitionKey(testDocument.pk));

// Test the exact scenario from the original issue
string longNumericString = "12345678901234567890"; // 20 characters - this was the problematic case
string testValue = "test_value_for_long_numeric_key";

// This should work now with the fix
List<PatchOperation> patchOperations = new List<PatchOperation>
{
PatchOperation.Add($"/strings/{longNumericString}", testValue)
};

// Execute the patch operation
ContainerInternal containerInternal = (ContainerInternal)this.Container;
var response = await containerInternal.PatchItemAsync<dynamic>(
id: testDocument.id,
partitionKey: new Cosmos.PartitionKey(testDocument.pk),
patchOperations: patchOperations);

// Verify the patch operation succeeded
Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);
Assert.IsNotNull(response.Resource);

// Read the document back to verify the patch was applied correctly
var readResponse = await this.Container.ReadItemAsync<dynamic>(
testDocument.id,
new Cosmos.PartitionKey(testDocument.pk));

Assert.AreEqual(HttpStatusCode.OK, readResponse.StatusCode);
Assert.IsNotNull(readResponse.Resource);

// Verify that the value was added to the strings object with the long numeric key
dynamic doc = readResponse.Resource;
Newtonsoft.Json.Linq.JObject stringsObj = doc.strings;

// The key should exist and have the correct value
Assert.IsTrue(stringsObj.ContainsKey(longNumericString));
Assert.AreEqual(testValue, stringsObj[longNumericString].ToString());

// Test with an even longer numeric string (30 characters)
string veryLongNumericString = "123456789012345678901234567890";
string testValue2 = "test_value_for_very_long_numeric_key";

patchOperations.Clear();
patchOperations.Add(PatchOperation.Add($"/strings/{veryLongNumericString}", testValue2));

response = await containerInternal.PatchItemAsync<dynamic>(
id: testDocument.id,
partitionKey: new Cosmos.PartitionKey(testDocument.pk),
patchOperations: patchOperations);

Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);

// Verify the second patch operation as well
readResponse = await this.Container.ReadItemAsync<dynamic>(
testDocument.id,
new Cosmos.PartitionKey(testDocument.pk));

doc = readResponse.Resource;
stringsObj = doc.strings;

Assert.IsTrue(stringsObj.ContainsKey(veryLongNumericString));
Assert.AreEqual(testValue2, stringsObj[veryLongNumericString].ToString());

// Test that shorter numeric strings still work (should not be affected by the fix)
string shortNumericString = "123456789"; // 9 characters
string testValue3 = "test_value_for_short_numeric_key";

patchOperations.Clear();
patchOperations.Add(PatchOperation.Add($"/strings/{shortNumericString}", testValue3));

response = await containerInternal.PatchItemAsync<dynamic>(
id: testDocument.id,
partitionKey: new Cosmos.PartitionKey(testDocument.pk),
patchOperations: patchOperations);

Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);

// Verify that short numeric strings still work correctly
readResponse = await this.Container.ReadItemAsync<dynamic>(
testDocument.id,
new Cosmos.PartitionKey(testDocument.pk));

doc = readResponse.Resource;
stringsObj = doc.strings;

Assert.IsTrue(stringsObj.ContainsKey(shortNumericString));
Assert.AreEqual(testValue3, stringsObj[shortNumericString].ToString());

// Test that mixed alphanumeric strings work (should not be affected)
string mixedString = "abc123456789012345678901234567890def";
string testValue4 = "test_value_for_mixed_key";

patchOperations.Clear();
patchOperations.Add(PatchOperation.Add($"/strings/{mixedString}", testValue4));

response = await containerInternal.PatchItemAsync<dynamic>(
id: testDocument.id,
partitionKey: new Cosmos.PartitionKey(testDocument.pk),
patchOperations: patchOperations);

Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);
}

[TestMethod]
[DataRow(false, DisplayName = "QueryItemAsyncTest - Binary encoding disabled.")]
[DataRow(true, DisplayName = "QueryItemAsyncTest - Binary encoding enabled.")]
public async Task QueryItemAsyncTest(bool binaryEncodingEnabled)
{
if (binaryEncodingEnabled)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
using System;
using System.Collections.Generic;
using Microsoft.Azure.Cosmos;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Microsoft.Azure.Cosmos.Tests
{
/// <summary>
/// Tests for the fix to handle long numeric strings in patch operation paths
/// </summary>
[TestClass]
public class PatchOperationLongNumericPathTests
{
[TestMethod]
public void TestLongNumericStringInPath_EscapedCorrectly()
{
// Test the exact scenario from the issue
var longNumericString = "12345678901234567890"; // 20 characters - should be escaped
var processedPath = PatchPathHelper.ProcessPath($"/strings/{longNumericString}");

Assert.AreEqual($"/strings/[\"{longNumericString}\"]", processedPath);
}

[TestMethod]
public void TestVeryLongNumericStringInPath_EscapedCorrectly()
{
// Test with an even longer numeric string
var veryLongNumericString = "123456789012345678901234567890"; // 30 characters
var processedPath = PatchPathHelper.ProcessPath($"/strings/{veryLongNumericString}");

Assert.AreEqual($"/strings/[\"{veryLongNumericString}\"]", processedPath);
}

[TestMethod]
public void TestShortNumericStringInPath_NotEscaped()
{
// Test with a short numeric string (should not be escaped)
var shortNumericString = "123456789"; // 9 characters
var processedPath = PatchPathHelper.ProcessPath($"/strings/{shortNumericString}");

Assert.AreEqual($"/strings/{shortNumericString}", processedPath);
}

[TestMethod]
public void TestBoundaryNumericStringInPath_NotEscaped()
{
// Test with a numeric string at the boundary (19 characters - should not be escaped)
var boundaryNumericString = "1234567890123456789"; // 19 characters
var processedPath = PatchPathHelper.ProcessPath($"/strings/{boundaryNumericString}");

Assert.AreEqual($"/strings/{boundaryNumericString}", processedPath);
}

[TestMethod]
public void TestMixedAlphaNumericStringInPath_NotEscaped()
{
// Test with mixed alphanumeric string (should not be escaped regardless of length)
var mixedString = "abc123456789012345678901234567890def";
var processedPath = PatchPathHelper.ProcessPath($"/strings/{mixedString}");

Assert.AreEqual($"/strings/{mixedString}", processedPath);
}

[TestMethod]
public void TestMultipleSegmentsWithLongNumeric_OnlyLongNumericEscaped()
{
// Test with multiple segments where only the long numeric one should be escaped
var path = "/strings/12345678901234567890/nested/123456789";
var processedPath = PatchPathHelper.ProcessPath(path);

Assert.AreEqual("/strings/[\"12345678901234567890\"]/nested/123456789", processedPath);
}

[TestMethod]
public void TestMultipleLongNumericSegments_AllEscaped()
{
// Test with multiple long numeric segments
var path = "/data/12345678901234567890/items/987654321098765432109876543210";
var processedPath = PatchPathHelper.ProcessPath(path);

Assert.AreEqual("/data/[\"12345678901234567890\"]/items/[\"987654321098765432109876543210\"]", processedPath);
}

[TestMethod]
public void TestPatchOperationsWithLongNumericPaths_CreatedSuccessfully()
{
// Test that patch operations can be created with long numeric paths
var longNumericString = "12345678901234567890";

var addOperation = PatchOperation.Add($"/strings/{longNumericString}", "test_value");
var replaceOperation = PatchOperation.Replace($"/data/{longNumericString}", "new_value");
var removeOperation = PatchOperation.Remove($"/items/{longNumericString}");

Assert.AreEqual($"/strings/{longNumericString}", addOperation.Path);
Assert.AreEqual($"/data/{longNumericString}", replaceOperation.Path);
Assert.AreEqual($"/items/{longNumericString}", removeOperation.Path);
}

[TestMethod]
public void TestEdgeCases_HandledCorrectly()
{
// Test edge cases
Assert.AreEqual("", PatchPathHelper.ProcessPath(""));
Assert.AreEqual("/", PatchPathHelper.ProcessPath("/"));
Assert.AreEqual(null, PatchPathHelper.ProcessPath(null));
Assert.AreEqual("/[\"12345678901234567890\"]", PatchPathHelper.ProcessPath("/12345678901234567890"));
}

[TestMethod]
public void TestMoveOperationWithLongNumericPaths_BothPathsEscaped()
{
// Test that move operations escape both 'from' and 'path' when they contain long numeric strings
var longNumericString1 = "12345678901234567890";
var longNumericString2 = "98765432109876543210";

var moveOperation = PatchOperation.Move($"/source/{longNumericString1}", $"/target/{longNumericString2}");

Assert.AreEqual($"/target/{longNumericString2}", moveOperation.Path);
Assert.AreEqual($"/source/{longNumericString1}", moveOperation.From);

// Test that the helper processes both paths correctly
var processedPath = PatchPathHelper.ProcessPath(moveOperation.Path);
var processedFrom = PatchPathHelper.ProcessPath(moveOperation.From);

Assert.AreEqual($"/target/[\"{longNumericString2}\"]", processedPath);
Assert.AreEqual($"/source/[\"{longNumericString1}\"]", processedFrom);
}
}
}
Loading