Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit aa808b7

Browse files
JonHannaVSadov
JonHanna
authored and
VSadov
committed
Have S.L.Expressions accept conversions it incorrectly rejects. (#25768)
* Have S.L.Expressions accept conversions it incorrectly rejects. Extend the conversion checks to allow array-related explicit reference that were rejected by Expression.Convert and Expression.CheckedConvert. Fixes #25760, and hence fixes #25754 * Move test case that fails on netfx into separate non-netfx test * CheckedConvert equivalent of tests * Future-proof s_arrayAssignableInterfaces Derive values for s_arrayAssignableInterfaces dynamically. Extra cost is only spent once, but it will automatically deal with any future additions to the generic interfaces supported by arrays.
1 parent 49d3084 commit aa808b7

File tree

5 files changed

+624
-3
lines changed

5 files changed

+624
-3
lines changed

‎src/Microsoft.CSharp/tests/ExplicitConversionTests.cs‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Runtime.CompilerServices;
8+
using Xunit;
9+
10+
namespace Microsoft.CSharp.RuntimeBinder.Tests
11+
{
12+
public class ExplicitConversionTests
13+
{
14+
private enum ExpectedConversionResult
15+
{
16+
Succeed,
17+
CompileError,
18+
RuntimeError
19+
}
20+
private static void AssertExplicitConvert<TSource, TTarget>(TSource argument, TTarget target, ExpectedConversionResult expected)
21+
{
22+
CallSiteBinder binder = Binder.Convert(CSharpBinderFlags.ConvertExplicit, typeof(TTarget), typeof(ExplicitConversionTests));
23+
CallSite<Func<CallSite, TSource, TTarget>> callSite =
24+
CallSite<Func<CallSite, TSource, TTarget>>.Create(binder);
25+
Func<CallSite, TSource, TTarget> func = callSite.Target;
26+
switch (expected)
27+
{
28+
case ExpectedConversionResult.CompileError:
29+
Assert.Throws<RuntimeBinderException>(() => func(callSite, argument));
30+
break;
31+
32+
case ExpectedConversionResult.RuntimeError:
33+
Assert.Throws<InvalidCastException>(() => func(callSite, argument));
34+
break;
35+
36+
default:
37+
TTarget result = func(callSite, argument);
38+
Assert.Equal(target, result);
39+
break;
40+
}
41+
42+
if (typeof(TSource) != typeof(object))
43+
{
44+
AssertExplicitConvert<object, TTarget>(argument, target, expected);
45+
}
46+
}
47+
48+
49+
private interface IInterface
50+
{
51+
}
52+
53+
private class UnsealedClass
54+
{
55+
}
56+
57+
private sealed class SealedClass
58+
{
59+
}
60+
61+
private class ImplementingClass : IInterface
62+
{
63+
}
64+
65+
private class ImplementingSealedClass : IInterface
66+
{
67+
}
68+
69+
private class UnrelatedNonInterface
70+
{
71+
}
72+
73+
private struct Struct
74+
{
75+
}
76+
77+
private struct ImplementingStruct : IInterface
78+
{
79+
}
80+
81+
[Fact]
82+
public void ClassInterfaceExplicitConversion()
83+
{
84+
AssertExplicitConvert(new SealedClass(), default(IInterface), ExpectedConversionResult.CompileError);
85+
AssertExplicitConvert(new UnsealedClass(), default(IInterface), ExpectedConversionResult.RuntimeError);
86+
ImplementingClass ic = new ImplementingClass();
87+
AssertExplicitConvert(ic, (IInterface)ic, ExpectedConversionResult.Succeed);
88+
ImplementingSealedClass isc = new ImplementingSealedClass();
89+
AssertExplicitConvert(isc, (IInterface)isc, ExpectedConversionResult.Succeed);
90+
AssertExplicitConvert(new Struct(), default(IInterface), ExpectedConversionResult.CompileError);
91+
AssertExplicitConvert(new ImplementingStruct(), new ImplementingStruct(), ExpectedConversionResult.Succeed);
92+
AssertExplicitConvert(new SealedClass(), default(UnrelatedNonInterface), ExpectedConversionResult.CompileError);
93+
AssertExplicitConvert(new UnsealedClass(), default(UnrelatedNonInterface), ExpectedConversionResult.CompileError);
94+
AssertExplicitConvert(new Struct(), default(UnrelatedNonInterface), ExpectedConversionResult.CompileError);
95+
}
96+
97+
[Fact]
98+
public void InterfaceClassExplicitConversion()
99+
{
100+
IInterface iif = new ImplementingClass();
101+
AssertExplicitConvert(iif, default(SealedClass), ExpectedConversionResult.CompileError);
102+
AssertExplicitConvert(iif, default(UnsealedClass), ExpectedConversionResult.CompileError);
103+
ImplementingClass ic = new ImplementingClass();
104+
AssertExplicitConvert((IInterface)ic, ic, ExpectedConversionResult.Succeed);
105+
ImplementingSealedClass isc = new ImplementingSealedClass();
106+
AssertExplicitConvert((IInterface)isc, isc, ExpectedConversionResult.Succeed);
107+
AssertExplicitConvert(iif, default(Struct), ExpectedConversionResult.CompileError);
108+
AssertExplicitConvert((IInterface)new ImplementingStruct(), new ImplementingStruct(), ExpectedConversionResult.Succeed);
109+
}
110+
111+
[Fact]
112+
public void ClassInterfaceArrayElementExplicitConversions()
113+
{
114+
AssertExplicitConvert(new SealedClass[0], default(IInterface[]), ExpectedConversionResult.CompileError);
115+
var ic = new ImplementingClass[0];
116+
AssertExplicitConvert(ic, (IInterface[])ic, ExpectedConversionResult.Succeed);
117+
var isc = new ImplementingSealedClass[0];
118+
AssertExplicitConvert(isc, (IInterface[])isc, ExpectedConversionResult.Succeed);
119+
AssertExplicitConvert(new Struct[0], default(IInterface[]), ExpectedConversionResult.CompileError);
120+
AssertExplicitConvert(new SealedClass[0], default(UnrelatedNonInterface[]), ExpectedConversionResult.CompileError);
121+
AssertExplicitConvert(new UnsealedClass[0], default(UnrelatedNonInterface[]), ExpectedConversionResult.CompileError);
122+
AssertExplicitConvert(new Struct[0], default(UnrelatedNonInterface[]), ExpectedConversionResult.CompileError);
123+
}
124+
125+
[Fact, SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "25754 is not fixed in NetFX")]
126+
public void ClassInterfaceArrayElementExplicitConversionsCoreFX()
127+
{
128+
AssertExplicitConvert(new UnsealedClass[0], default(IInterface[]), ExpectedConversionResult.RuntimeError);
129+
}
130+
131+
[Fact]
132+
public void ClassInterfaceArrayIListElementExplicitConversions()
133+
{
134+
AssertExplicitConvert(new SealedClass[0], default(IList<IInterface>), ExpectedConversionResult.CompileError);
135+
AssertExplicitConvert(new UnsealedClass[0], default(IList<IInterface>), ExpectedConversionResult.RuntimeError);
136+
var ic = new ImplementingClass[0];
137+
AssertExplicitConvert(ic, (IList<IInterface>)ic, ExpectedConversionResult.Succeed);
138+
var isc = new ImplementingSealedClass[0];
139+
AssertExplicitConvert(isc, (IList<IInterface>)isc, ExpectedConversionResult.Succeed);
140+
AssertExplicitConvert(new Struct[0], default(IList<IInterface>), ExpectedConversionResult.CompileError);
141+
AssertExplicitConvert(new SealedClass[0], default(IList<UnrelatedNonInterface>), ExpectedConversionResult.CompileError);
142+
AssertExplicitConvert(new UnsealedClass[0], default(IList<UnrelatedNonInterface>), ExpectedConversionResult.CompileError);
143+
AssertExplicitConvert(new Struct[0], default(IList<UnrelatedNonInterface>), ExpectedConversionResult.CompileError);
144+
}
145+
}
146+
}

‎src/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
<Compile Include="AssignmentTests.cs" />
1414
<Compile Include="BindingErrors.cs" />
1515
<Compile Include="EnumUnaryOperationTests.cs" />
16+
<Compile Include="ExplicitConversionTests.cs" />
1617
<Compile Include="ImplicitConversionTests.cs" />
1718
<Compile Include="CSharpArgumentInfoTests.cs" />
1819
<Compile Include="DefaultParameterTests.cs" />
@@ -34,4 +35,4 @@
3435
<Compile Include="AccessTests.netcoreapp.cs" />
3536
</ItemGroup>
3637
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
37-
</Project>
38+
</Project>

‎src/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs‎

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Diagnostics;
6+
using System.Linq;
67
using System.Linq.Expressions;
78
using System.Reflection;
89

910
namespace System.Dynamic.Utils
1011
{
1112
internal static class TypeUtils
1213
{
14+
private static readonly Type[] s_arrayAssignableInterfaces = typeof(int[]).GetInterfaces()
15+
.Where(i => i.IsGenericType)
16+
.Select(i => i.GetGenericTypeDefinition())
17+
.ToArray();
18+
1319
public static Type GetNonNullableType(this Type type) => IsNullableType(type) ? type.GetGenericArguments()[0] : type;
1420

1521
public static Type GetNullableType(this Type type)
@@ -274,8 +280,139 @@ public static bool HasReferenceConversionTo(this Type source, Type dest)
274280
return true;
275281
}
276282

277-
// Object conversion
278-
return source == typeof(object) || dest == typeof(object);
283+
// Object conversion handled by assignable above.
284+
Debug.Assert(source != typeof(object) && dest != typeof(object));
285+
286+
return (source.IsArray || dest.IsArray) && StrictHasReferenceConversionTo(source, dest, true);
287+
}
288+
289+
private static bool StrictHasReferenceConversionTo(this Type source, Type dest, bool skipNonArray)
290+
{
291+
// HasReferenceConversionTo was both too strict and too lax. It was too strict in prohibiting
292+
// some valid conversions involving arrays, and too lax in allowing casts between interfaces
293+
// and sealed classes that don't implement them. Unfortunately fixing the lax cases would be
294+
// a breaking change, especially since such expressions will even work if only given null
295+
// arguments.
296+
// This method catches the cases that were incorrectly disallowed, but when it needs to
297+
// examine possible conversions of element or type parameters it applies stricter rules.
298+
299+
for(;;)
300+
{
301+
if (!skipNonArray) // Skip if we just came from HasReferenceConversionTo and have just tested these
302+
{
303+
if (source.IsValueType | dest.IsValueType)
304+
{
305+
return false;
306+
}
307+
308+
// Includes to case of either being typeof(object)
309+
if (source.IsAssignableFrom(dest) || dest.IsAssignableFrom(source))
310+
{
311+
return true;
312+
}
313+
314+
if (source.IsInterface)
315+
{
316+
if (dest.IsInterface || dest.IsClass && !dest.IsSealed)
317+
{
318+
return true;
319+
}
320+
}
321+
else if (dest.IsInterface)
322+
{
323+
if (source.IsClass && !source.IsSealed)
324+
{
325+
return true;
326+
}
327+
}
328+
}
329+
330+
if (source.IsArray)
331+
{
332+
if (dest.IsArray)
333+
{
334+
if (source.GetArrayRank() != dest.GetArrayRank() || source.IsSZArray != dest.IsSZArray)
335+
{
336+
return false;
337+
}
338+
339+
source = source.GetElementType();
340+
dest = dest.GetElementType();
341+
skipNonArray = false;
342+
}
343+
else
344+
{
345+
return HasArrayToInterfaceConversion(source, dest);
346+
}
347+
}
348+
else if (dest.IsArray)
349+
{
350+
if (HasInterfaceToArrayConversion(source, dest))
351+
{
352+
return true;
353+
}
354+
355+
return IsImplicitReferenceConversion(typeof(Array), source);
356+
}
357+
else
358+
{
359+
return IsLegalExplicitVariantDelegateConversion(source, dest);
360+
}
361+
}
362+
}
363+
364+
private static bool HasArrayToInterfaceConversion(Type source, Type dest)
365+
{
366+
Debug.Assert(source.IsArray);
367+
if (!source.IsSZArray || !dest.IsInterface || !dest.IsGenericType)
368+
{
369+
return false;
370+
}
371+
372+
Type[] destParams = dest.GetGenericArguments();
373+
if (destParams.Length != 1)
374+
{
375+
return false;
376+
}
377+
378+
Type destGen = dest.GetGenericTypeDefinition();
379+
380+
foreach (Type iface in s_arrayAssignableInterfaces)
381+
{
382+
if (AreEquivalent(destGen, iface))
383+
{
384+
return StrictHasReferenceConversionTo(source.GetElementType(), destParams[0], false);
385+
}
386+
}
387+
388+
return false;
389+
}
390+
391+
private static bool HasInterfaceToArrayConversion(Type source, Type dest)
392+
{
393+
Debug.Assert(dest.IsSZArray);
394+
if (!dest.IsSZArray || !source.IsInterface || !source.IsGenericType)
395+
{
396+
return false;
397+
}
398+
399+
Type[] sourceParams = source.GetGenericArguments();
400+
if (sourceParams.Length != 1)
401+
{
402+
return false;
403+
}
404+
405+
Type sourceGen = source.GetGenericTypeDefinition();
406+
407+
foreach (Type iface in s_arrayAssignableInterfaces)
408+
{
409+
if (AreEquivalent(sourceGen, iface))
410+
{
411+
return StrictHasReferenceConversionTo(sourceParams[0], dest.GetElementType(), false);
412+
}
413+
}
414+
415+
return false;
279416
}
280417

281418
private static bool IsCovariant(Type t)

0 commit comments

Comments
 (0)