Skip to content

Commit c574472

Browse files
authored
Only allow assignments of explicitly supported types to DynamicData (#36791)
* Preparation for Core release 1.33.0 * Add allow list converter * Add test; plumb through * support anonymous types * Add todo list of types to support; remaining work is reference semantics * Add more test cases; refactor a bit * Add Etag to allow list * add TODOs about reference semantics * Refactor * Back-out early serialization for type validation instead * pr fb * Move type validation to dynamic layer * refactor * refactor * Move naming enum to Azure.Core.Serialization * Rename to PropertyNameConversion * Add dynamic options to response content * Plumb through response content options * Update ClientOptions API * Enable it in DynamicData * RequestContent and CLU * Update docs and samples * Unify request and response content * Add TODO * Rename internal enum * pr fb * nit * undo * missed update * more updates from pr fb * Initial (messy) collection support. * Refactor to type-only check as a first step * Add placeholder value check * Initial collection value check, non-poco * nit tidy * architect API feedback * clu * refactor pipeline bits to use ProtocolOptions (remove dependency on types in Dynamic namespace) * Update set behavior as discussed in arch board * nits * PR fb * clu * revert prepare release change * In TDD spirit, add failing tests * fix * Initial work on value semantics * nit and fix * nits * fix * make value semantics tests pass * delete extraneous code: * updates * add validation for MJE assignment * pr fb * updates post-merge * pr fb * fix bug in POCO type * Add tests of assignment * add tests * Allow anonymous types but not POCOs * disallow IEnumerable * Remove object from allow list * remove AllowListConverter per net7.0 bug * Don't allow heterogeneous collections * anonymous types * don't allocate collections for added and removed properties * pr fb * renames * Update MJD tests per value semantics * Rework primitive type check * Simplify anonymous type check * Don't allocate HashSet in allow list * reallocate visited list more occasionally * pr fb * remove cycle detection and simplify; add tests * simplify further * more tests of enumerables with changes * one more test * Updates post merge
1 parent a30ff1c commit c574472

21 files changed

+2003
-181
lines changed
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections;
6+
using System.Collections.Generic;
7+
using System.Reflection;
8+
using System.Text.Json;
9+
10+
namespace Azure.Core.Dynamic
11+
{
12+
public partial class DynamicData
13+
{
14+
internal class AllowList
15+
{
16+
public static void AssertAllowedValue<T>(T value)
17+
{
18+
if (value == null)
19+
{
20+
return;
21+
}
22+
23+
if (!IsAllowedValue(value))
24+
{
25+
throw new NotSupportedException($"Assigning this value is not supported, either because its type '{value.GetType()}' is not allowed, or because it contains unallowed types.");
26+
}
27+
}
28+
29+
private static bool IsAllowedValue<T>(T value)
30+
{
31+
if (value == null)
32+
{
33+
return true;
34+
}
35+
36+
Type type = value.GetType();
37+
if (IsAllowedType(type))
38+
{
39+
return true;
40+
}
41+
42+
if (IsAllowedCollectionValue(type, value))
43+
{
44+
return true;
45+
}
46+
47+
return IsAllowedAnonymousValue(type, value);
48+
}
49+
50+
private static bool IsAllowedType(Type type)
51+
{
52+
return type.IsPrimitive ||
53+
type == typeof(decimal) ||
54+
type == typeof(string) ||
55+
type == typeof(DateTime) ||
56+
type == typeof(DateTimeOffset) ||
57+
type == typeof(TimeSpan) ||
58+
type == typeof(Uri) ||
59+
type == typeof(Guid) ||
60+
type == typeof(ETag) ||
61+
type == typeof(JsonElement) ||
62+
type == typeof(JsonDocument) ||
63+
type == typeof(DynamicData);
64+
}
65+
66+
private static bool IsAllowedCollectionValue<T>(Type type, T value)
67+
{
68+
return
69+
IsAllowedArrayValue(type, value) ||
70+
IsAllowedListValue(type, value) ||
71+
IsAllowedDictionaryValue(type, value);
72+
}
73+
74+
private static bool IsAllowedArrayValue<T>(Type type, T value)
75+
{
76+
if (value is not Array array)
77+
{
78+
return false;
79+
}
80+
81+
Type? elementType = type.GetElementType();
82+
if (elementType == null)
83+
{
84+
return false;
85+
}
86+
87+
if (elementType.IsPrimitive || elementType == typeof(string))
88+
{
89+
return true;
90+
}
91+
92+
return IsAllowedEnumerableValue(elementType, array);
93+
}
94+
95+
private static bool IsAllowedListValue<T>(Type type, T value)
96+
{
97+
if (value == null)
98+
{
99+
return true;
100+
}
101+
102+
if (!type.IsGenericType)
103+
{
104+
return false;
105+
}
106+
107+
if (type.GetGenericTypeDefinition() != typeof(List<>))
108+
{
109+
return false;
110+
}
111+
112+
Type genericArgument = type.GetGenericArguments()[0];
113+
if (genericArgument.IsPrimitive || genericArgument == typeof(string))
114+
{
115+
return true;
116+
}
117+
118+
return IsAllowedEnumerableValue(genericArgument, (IEnumerable)value);
119+
}
120+
121+
private static bool IsAllowedDictionaryValue<T>(Type type, T value)
122+
{
123+
if (value == null)
124+
{
125+
return true;
126+
}
127+
128+
if (!type.IsGenericType)
129+
{
130+
return false;
131+
}
132+
133+
if (type.GetGenericTypeDefinition() != typeof(Dictionary<,>))
134+
{
135+
return false;
136+
}
137+
138+
Type[] genericArguments = type.GetGenericArguments();
139+
if (genericArguments[0] != typeof(string))
140+
{
141+
return false;
142+
}
143+
144+
if (genericArguments[1].IsPrimitive || genericArguments[1] == typeof(string))
145+
{
146+
return true;
147+
}
148+
149+
return IsAllowedEnumerableValue(genericArguments[1], ((IDictionary)value).Values);
150+
}
151+
152+
private static bool IsAllowedEnumerableValue(Type elementType, IEnumerable enumerable)
153+
{
154+
foreach (var item in enumerable)
155+
{
156+
if (item == null)
157+
{
158+
continue;
159+
}
160+
161+
// Don't allow heterogenous collections
162+
if (item.GetType() != elementType)
163+
{
164+
return false;
165+
}
166+
167+
if (!IsAllowedValue(item))
168+
{
169+
return false;
170+
}
171+
}
172+
173+
return true;
174+
}
175+
176+
private static bool IsAllowedAnonymousValue<T>(Type type, T value)
177+
{
178+
if (!IsAnonymousType(type))
179+
{
180+
return false;
181+
}
182+
183+
foreach (PropertyInfo property in type.GetProperties())
184+
{
185+
if (!IsAllowedValue(property.GetValue(value)))
186+
{
187+
return false;
188+
}
189+
}
190+
191+
return true;
192+
}
193+
194+
private static bool IsAnonymousType(Type type)
195+
{
196+
return type.Name.StartsWith("<>f__AnonymousType");
197+
}
198+
}
199+
}
200+
}

sdk/core/Azure.Core/src/DynamicData/DynamicData.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace Azure.Core.Dynamic
2121
/// This and related types are not intended to be mocked.
2222
/// </summary>
2323
[DebuggerDisplay("{DebuggerDisplay,nq}")]
24-
[JsonConverter(typeof(JsonConverter))]
24+
[JsonConverter(typeof(DynamicDataJsonConverter))]
2525
public sealed partial class DynamicData : IDisposable
2626
{
2727
private static readonly MethodInfo GetPropertyMethod = typeof(DynamicData).GetMethod(nameof(GetProperty), BindingFlags.NonPublic | BindingFlags.Instance)!;
@@ -138,6 +138,7 @@ private IEnumerable GetEnumerable()
138138
private object? SetProperty(string name, object value)
139139
{
140140
Argument.AssertNotNullOrEmpty(name, nameof(name));
141+
AllowList.AssertAllowedValue(value);
141142

142143
if (HasTypeConverter(value))
143144
{
@@ -174,6 +175,8 @@ private object ConvertType(object value)
174175

175176
private object? SetViaIndexer(object index, object value)
176177
{
178+
AllowList.AssertAllowedValue(value);
179+
177180
switch (index)
178181
{
179182
case string propertyName:
@@ -342,7 +345,7 @@ public override int GetHashCode()
342345
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
343346
private string DebuggerDisplay => _element.DebuggerDisplay;
344347

345-
private class JsonConverter : JsonConverter<DynamicData>
348+
private class DynamicDataJsonConverter : JsonConverter<DynamicData>
346349
{
347350
public override DynamicData Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
348351
{

sdk/core/Azure.Core/src/DynamicData/DynamicDataOptions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
using System.Collections.Generic;
54
using System.Linq;
65
using System.Text.Json;
76
using System.Text.Json.Serialization;
Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,39 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using System;
45
using System.Text.Json;
56

67
namespace Azure.Core.Json
78
{
89
internal struct MutableJsonChange
910
{
10-
private readonly JsonSerializerOptions _serializerOptions;
1111
private JsonElement? _serializedValue;
12+
private readonly JsonSerializerOptions _serializerOptions;
1213

13-
public MutableJsonChange(string path, int index, object? value, bool replacesJsonElement, JsonSerializerOptions options)
14+
public MutableJsonChange(string path,
15+
int index,
16+
object? value,
17+
JsonSerializerOptions options,
18+
MutableJsonChangeKind changeKind,
19+
string? addedPropertyName)
1420
{
1521
Path = path;
1622
Index = index;
1723
Value = value;
18-
ReplacesJsonElement = replacesJsonElement;
1924
_serializerOptions = options;
25+
ChangeKind = changeKind;
26+
AddedPropertyName = addedPropertyName;
27+
28+
if (value is JsonElement element)
29+
{
30+
_serializedValue = element;
31+
}
32+
33+
if (value is JsonDocument doc)
34+
{
35+
_serializedValue = doc.RootElement;
36+
}
2037
}
2138

2239
public string Path { get; }
@@ -25,39 +42,57 @@ public MutableJsonChange(string path, int index, object? value, bool replacesJso
2542

2643
public object? Value { get; }
2744

28-
/// <summary>
29-
/// The change invalidates the existing node's JsonElement
30-
/// due to changes in JsonValueKind or path structure.
31-
/// If this is true, Value holds a new JsonElement.
32-
/// </summary>
33-
public bool ReplacesJsonElement { get; }
45+
public string? AddedPropertyName { get; }
46+
47+
public MutableJsonChangeKind ChangeKind { get; }
48+
49+
public JsonValueKind ValueKind => GetSerializedValue().ValueKind;
3450

35-
internal JsonElement AsJsonElement()
51+
internal JsonElement GetSerializedValue()
3652
{
3753
if (_serializedValue != null)
3854
{
3955
return _serializedValue.Value;
4056
}
4157

42-
if (Value is JsonElement element)
43-
{
44-
_serializedValue = element;
45-
return element;
46-
}
47-
4858
byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(Value, _serializerOptions);
4959
_serializedValue = JsonDocument.Parse(bytes).RootElement;
5060
return _serializedValue.Value;
5161
}
5262

63+
internal bool IsDescendant(string path)
64+
{
65+
if (path.Length > 0)
66+
{
67+
// Restrict matches (e.g. so we don't think 'a' is a parent of 'abc').
68+
path += MutableJsonDocument.ChangeTracker.Delimiter;
69+
}
70+
71+
return Path.StartsWith(path, StringComparison.Ordinal);
72+
}
73+
74+
internal bool IsDirectDescendant(string path)
75+
{
76+
if (!IsDescendant(path))
77+
{
78+
return false;
79+
}
80+
81+
string[] ancestorPath = path.Split(MutableJsonDocument.ChangeTracker.Delimiter);
82+
int ancestorPathLength = string.IsNullOrEmpty(ancestorPath[0]) ? 0 : ancestorPath.Length;
83+
int descendantPathLength = Path.Split(MutableJsonDocument.ChangeTracker.Delimiter).Length;
84+
85+
return ancestorPathLength == (descendantPathLength - 1);
86+
}
87+
5388
internal string AsString()
5489
{
55-
return AsJsonElement().ToString() ?? "null";
90+
return GetSerializedValue().ToString() ?? "null";
5691
}
5792

5893
public override string ToString()
5994
{
60-
return $"Path={Path}; Value={Value}; ReplacesJsonElement={ReplacesJsonElement}";
95+
return $"Path={Path}; Value={Value}; Kind={ValueKind}; ChangeKind={ChangeKind}";
6196
}
6297
}
6398
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
namespace Azure.Core.Json
5+
{
6+
internal enum MutableJsonChangeKind
7+
{
8+
PropertyValue,
9+
PropertyAddition,
10+
PropertyRemoval,
11+
}
12+
}

0 commit comments

Comments
 (0)