Ensuring Url safe string key values. Aligning with ODL Client. Fixes O…

…Data#1390.
uffelauesen · Jan 20, 2025 · a9a055e · a9a055e
1 parent 275985b
commit a9a055e
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 7 deletions.
diff --git a/src/Microsoft.AspNetCore.OData/Routing/ODataPathSegmentHandler.cs b/src/Microsoft.AspNetCore.OData/Routing/ODataPathSegmentHandler.cs
@@ -318,7 +318,7 @@ internal static string ConvertKeysToString(IEnumerable<KeyValuePair<string, obje
                 return string.Join(
                     ",",
                     keyValuePairs.Select(keyValuePair =>
-                        TranslateNode(keyValuePair.Value).EscapeBackSlashUriString()).ToArray());
+                        TranslateNode(keyValuePair.Value)).ToArray());
             }
         }
 
@@ -328,7 +328,7 @@ internal static string ConvertKeysToString(IEnumerable<KeyValuePair<string, obje
             keyValuePairs.Select(keyValuePair =>
                 (keyValuePair.Key +
                  "=" +
-                 TranslateNode(keyValuePair.Value).EscapeBackSlashUriString())).ToArray());
+                 TranslateNode(keyValuePair.Value))).ToArray());
     }
 
     internal static string TranslateNode(object node)
@@ -364,6 +364,17 @@ internal static string TranslateNode(object node)
             return parameterAliasNode.Alias;
         }
 
-        return ODataUriUtils.ConvertToUriLiteral(node, ODataVersion.V4);
+        string uriLiteral = ODataUriUtils.ConvertToUriLiteral(node, ODataVersion.V4);
+
+        if (node is string && uriLiteral.Length > 2)
+        {
+            // ODataUriUtils.ConvertToUriLiteral does not encoded the value.
+            // The result for keys to use on the wire (like odata.id, odata.readlink, odata.editLink and Location header) should be encoded,
+            // but still wrapped in unencoded '
+            // This to allign with how ODLs UriParser construct OData ID Uri's
+            return '\'' + Uri.EscapeDataString(uriLiteral.Substring(1, uriLiteral.Length - 2)) + '\'';
+        }
+
+        return uriLiteral;
     }
 }
diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/LocationHeaderTests.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/LocationHeaderTests.cs
@@ -58,7 +58,7 @@ public async Task CreateCustomerWithSingleKey_ReturnsCorrectLocationHeaderEscape
 
         string locationHeader = response.Headers.GetValues("Location").Single();
 
-        Assert.Equal("http://localhost/location/Customers('abc%2F$+%2F-8')", locationHeader);
+        Assert.Equal("http://localhost/location/Customers('abc%2F%24%2B%2F-8%27%27%20%26%2C%3F%22')", locationHeader);
     }
 
     [Fact]
@@ -101,7 +101,7 @@ public class HandleController : ODataController
     [HttpPost("location/customers")]
     public IActionResult CreateCustomer([FromBody]LocCustomer customer)
     {
-        customer.Id = $"{customer.Name}/$+/-8"; // insert slash middle
+        customer.Id = $"{customer.Name}/$+/-8' &,?\""; // insert slash middle and other unsafe url chars
         return Created(customer);
     }
 

diff --git a/test/Microsoft.AspNetCore.OData.Tests/Routing/ODataPathSegmentHandlerTests.cs b/test/Microsoft.AspNetCore.OData.Tests/Routing/ODataPathSegmentHandlerTests.cs
@@ -402,14 +402,60 @@ public void ConvertKeysToString_ConvertKeysValues_ShouldEscapeUriString()
         IEnumerable<KeyValuePair<string, object>> keys = new KeyValuePair<string, object>[]
         {
             KeyValuePair.Create("Id", (object)4),
-            KeyValuePair.Create("Name", (object)"2425/&Foo")
+            KeyValuePair.Create("Name", (object)"'24 25/&Foo,?\"")
         };
 
         // Act
         string actual = ODataPathSegmentHandler.ConvertKeysToString(keys, entityType);
 
         // Assert
-        Assert.Equal("Id=4,Name='2425%2F&Foo'", actual);
+        Assert.Equal("Id=4,Name='%27%2724%2025%2F%26Foo%2C%3F%22'", actual);
+    }
+
+    [Fact]
+    public void ConvertKeysToString_ConvertKeysValues_ProducesSameEncodingAsODL()
+    {
+        // Arrange
+        EdmEntityType entityType = new EdmEntityType("NS", "Entity");
+        IEdmStructuralProperty key1 = entityType.AddStructuralProperty("Id", EdmCoreModel.Instance.GetInt32(false));
+        IEdmStructuralProperty key2 = entityType.AddStructuralProperty("Name", EdmCoreModel.Instance.GetString(false));
+
+        entityType.AddKeys(key1, key2);
+        IEnumerable<KeyValuePair<string, object>> keys = new KeyValuePair<string, object>[]
+        {
+            KeyValuePair.Create("Id", (object)4),
+            KeyValuePair.Create("Name", (object)"'24 25/&Foo,?\"")
+        };
+
+        // Act
+        string actual = '(' + ODataPathSegmentHandler.ConvertKeysToString(keys, entityType) + ')';
+
+        ODataPath path = new ODataPath(new KeySegment(keys, entityType, null));
+
+        // Assert
+        Assert.Equal(path.ToResourcePathString(ODataUrlKeyDelimiter.Parentheses), actual);
+    }
+
+    [Fact]
+    public void ConvertKeysToString_ConvertKeysValues_ShouldNotQuoteNull()
+    {
+        // Arrange
+        EdmEntityType entityType = new EdmEntityType("NS", "Entity");
+        IEdmStructuralProperty key1 = entityType.AddStructuralProperty("Id", EdmCoreModel.Instance.GetInt32(false));
+        IEdmStructuralProperty key2 = entityType.AddStructuralProperty("Name", EdmCoreModel.Instance.GetString(true));
+
+        entityType.AddKeys(key1, key2);
+        IEnumerable<KeyValuePair<string, object>> keys = new KeyValuePair<string, object>[]
+        {
+            KeyValuePair.Create("Id", (object)4),
+            KeyValuePair.Create("Name", (object)null)
+        };
+
+        // Act
+        string actual = ODataPathSegmentHandler.ConvertKeysToString(keys, entityType);
+
+        // Assert
+        Assert.Equal("Id=4,Name=null", actual);
     }
 
     [Fact]