Skip to content

Commit

Permalink
Remove hardcoded limit in deserialization constructor arguments (dotn…
Browse files Browse the repository at this point in the history
…et#75982)

* Remove hardcoded limit in deserialization constructor arguments.

* Use correct opcode for indices >= 128

* Remove unneeeded changes

* Add reflection fallback in interpreted runtimes

* Revert "Add reflection fallback in interpreted runtimes"

This reverts commit e78ea20c532860d21bd0540a97ebfd1cf02370e3.

* disable test in interpreted mono

* Revert "disable test in interpreted mono"

This reverts commit 53f7a91.
  • Loading branch information
eiriktsarpalis authored Sep 26, 2022
1 parent f8b032c commit 3f0106a
Show file tree
Hide file tree
Showing 13 changed files with 240 additions and 51 deletions.
3 changes: 0 additions & 3 deletions src/libraries/System.Text.Json/Common/JsonConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ namespace System.Text.Json
{
internal static partial class JsonConstants
{
// The maximum number of parameters a constructor can have where it can be supported by the serializer.
public const int MaxParameterCount = 64;

// Standard format for double and single on non-inbox frameworks.
public const string DoubleFormatString = "G17";
public const string SingleFormatString = "G9";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -951,11 +951,6 @@ private static string GetParameterizedCtorInvocationFunc(TypeGenerationSpec type
int paramCount = parameters.Length;
Debug.Assert(paramCount != 0);

if (paramCount > JsonConstants.MaxParameterCount)
{
return "null";
}

const string ArgsVarName = "args";
int lastIndex = paramCount - 1;

Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,6 @@
<data name="ConstructorParamIncompleteBinding" xml:space="preserve">
<value>Each parameter in the deserialization constructor on type '{0}' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. Fields are only considered when 'JsonSerializerOptions.IncludeFields' is enabled. The match can be case-insensitive.</value>
</data>
<data name="ConstructorMaxOf64Parameters" xml:space="preserve">
<value>The deserialization constructor on type '{0}' may not have more than 64 parameters for deserialization.</value>
</data>
<data name="ObjectWithParameterizedCtorRefMetadataNotSupported" xml:space="preserve">
<value>Reference metadata is not supported when deserializing constructor parameters. See type '{0}'.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,13 @@ protected sealed override bool ReadAndCacheConstructorArgument(scoped ref ReadSt

protected sealed override object CreateObject(ref ReadStackFrame frame)
{
object[] arguments = (object[])frame.CtorArgumentState!.Arguments;
frame.CtorArgumentState.Arguments = null!;
Debug.Assert(frame.CtorArgumentState != null);
Debug.Assert(frame.JsonTypeInfo.CreateObjectWithArgs != null);

var createObject = (Func<object[], T>?)frame.JsonTypeInfo.CreateObjectWithArgs;
object[] arguments = (object[])frame.CtorArgumentState.Arguments;
frame.CtorArgumentState.Arguments = null!;

if (createObject == null)
{
// This means this constructor has more than 64 parameters.
ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(TypeToConvert);
}
Func<object[], T> createObject = (Func<object[], T>)frame.JsonTypeInfo.CreateObjectWithArgs;

object obj = createObject(arguments);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal abstract class MemberAccessor
public abstract Func<object>? CreateConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType);

public abstract Func<object[], T>? CreateParameterizedConstructor<T>(ConstructorInfo constructor);
public abstract Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor);

public abstract JsonTypeInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>?
CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo constructor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerab
=> s_cache.GetOrAdd((nameof(CreateImmutableEnumerableCreateRangeDelegate), typeof((TCollection, TElement)), null),
static (_) => s_sourceAccessor.CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>());

public override Func<object[], T>? CreateParameterizedConstructor<T>(ConstructorInfo constructor)
public override Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor)
=> s_cache.GetOrAdd((nameof(CreateParameterizedConstructor), typeof(T), constructor), static key => s_sourceAccessor.CreateParameterizedConstructor<T>((ConstructorInfo)key.member!));

public override JsonTypeInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>? CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo constructor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor

generator.Emit(OpCodes.Ret);

return (Func<object>)dynamicMethod.CreateDelegate(typeof(Func<object>));
return CreateDelegate<Func<object>>(dynamicMethod);
}

public override Func<object[], T>? CreateParameterizedConstructor<T>(ConstructorInfo constructor) =>
public override Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor) =>
CreateDelegate<Func<object[], T>>(CreateParameterizedConstructor(constructor));

private static DynamicMethod? CreateParameterizedConstructor(ConstructorInfo constructor)
private static DynamicMethod CreateParameterizedConstructor(ConstructorInfo constructor)
{
Type? type = constructor.DeclaringType;

Expand All @@ -76,11 +76,6 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
ParameterInfo[] parameters = constructor.GetParameters();
int parameterCount = parameters.Length;

if (parameterCount > JsonConstants.MaxParameterCount)
{
return null;
}

var dynamicMethod = new DynamicMethod(
ConstructorInfo.ConstructorName,
type,
Expand All @@ -95,7 +90,7 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
Type paramType = parameters[i].ParameterType;

generator.Emit(OpCodes.Ldarg_0);
generator.Emit(OpCodes.Ldc_I4_S, i);
generator.Emit(OpCodes.Ldc_I4, i);
generator.Emit(OpCodes.Ldelem_Ref);
generator.Emit(OpCodes.Unbox_Any, paramType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public ConstructorContext([DynamicallyAccessedMembers(DynamicallyAccessedMemberT
return new ConstructorContext(type).CreateInstance!;
}

public override Func<object[], T>? CreateParameterizedConstructor<T>(ConstructorInfo constructor)
public override Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor)
{
Type type = typeof(T);

Expand All @@ -50,11 +50,6 @@ public ConstructorContext([DynamicallyAccessedMembers(DynamicallyAccessedMemberT

int parameterCount = constructor.GetParameters().Length;

if (parameterCount > JsonConstants.MaxParameterCount)
{
return null;
}

return (arguments) =>
{
// The input array was rented from the shared ArrayPool, so its size is likely to be larger than the param count.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ public static void ThrowNotSupportedException_TypeRequiresAsyncSerialization(Typ
throw new NotSupportedException(SR.Format(SR.TypeRequiresAsyncSerialization, propertyType));
}

[DoesNotReturn]
public static void ThrowNotSupportedException_ConstructorMaxOf64Parameters(Type type)
{
throw new NotSupportedException(SR.Format(SR.ConstructorMaxOf64Parameters, type));
}

[DoesNotReturn]
public static void ThrowNotSupportedException_DictionaryKeyTypeNotSupported(Type keyType, JsonConverter converter)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ async Task RunTestAsync<T>()
}

[Fact]
public async Task Cannot_Deserialize_ObjectWith_Ctor_With_65_Params()
public async Task Can_Deserialize_ObjectWith_Ctor_With_65_Params()
{
async Task RunTestAsync<T>()
{
Expand All @@ -752,11 +752,8 @@ async Task RunTestAsync<T>()
sb.Append("Int32");
sb.Append(")");

NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper<T>(input));
Assert.Contains(type.ToString(), ex.ToString());

ex = await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper<T>("{}"));
Assert.Contains(type.ToString(), ex.ToString());
T value = await Serializer.DeserializeWrapper<T>(input);
value = await Serializer.DeserializeWrapper<T>("{}");
}

await RunTestAsync<Class_With_Ctor_With_65_Params>();
Expand Down Expand Up @@ -1426,6 +1423,17 @@ public async Task TestClassWithDefaultCtorParams()
JsonTestHelper.AssertJsonEqual(json, await Serializer.SerializeWrapper(obj));
}

[Fact]
public async Task TestClassWithManyConstructorParameters()
{
ClassWithManyConstructorParameters value = ClassWithManyConstructorParameters.Create();
string json = JsonSerializer.Serialize(value);

ClassWithManyConstructorParameters result = await Serializer.DeserializeWrapper<ClassWithManyConstructorParameters>(json);

Assert.Equal(value, result); // Type is C# record that implements structural equality.
}

public class ClassWithDefaultCtorParams
{
public Point_2D_Struct_WithAttribute Struct { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async Task RunTestAsync<T>(string testData)
}

[Fact]
public async Task Cannot_DeserializeAsync_ObjectWith_Ctor_With_65_Params()
public async Task Can_DeserializeAsync_ObjectWith_Ctor_With_65_Params()
{
if (StreamingSerializer is null)
{
Expand All @@ -137,6 +137,7 @@ async Task RunTestAsync<T>()
sb.Append("}");

string input = sb.ToString();
T value;

using (MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes(input)))
{
Expand All @@ -145,7 +146,7 @@ async Task RunTestAsync<T>()
DefaultBufferSize = 1
};

await Assert.ThrowsAsync<NotSupportedException>(async () => await StreamingSerializer.DeserializeWrapper<T>(stream, options));
value = await StreamingSerializer.DeserializeWrapper<T>(stream, options);
}

using (MemoryStream stream = new MemoryStream("{}"u8.ToArray()))
Expand All @@ -155,7 +156,7 @@ async Task RunTestAsync<T>()
DefaultBufferSize = 1
};

await Assert.ThrowsAsync<NotSupportedException>(async () => await StreamingSerializer.DeserializeWrapper<T>(stream, options));
value = await StreamingSerializer.DeserializeWrapper<T>(stream, options);
}
}

Expand Down
Loading

0 comments on commit 3f0106a

Please sign in to comment.