Catch InvalidCastException properly in Collection (dotnet/coreclr#25953)

The explicit implementations of IList.Add/Insert/Set methods catch
InvalidCastException for casting input argument to type T to throw
an ArgumentException. The issue here is they put the core collection
modification operation in the try block, which could be overridden
by sub-classes and produce an InvalidCastException from user logic,
even the argument passed to IList.Add/Insert/Set is valid.

To fix the issue, we just need to move the collection modification
outside the try block, and leave the casting in try block only. In
this way, we'll get an ArgumentException when the argument type is
invalid, but propagate the InvalidCastException thrown from user
code as it should be.

dotnet/corefxdotnet/coreclr#39919

Commit migrated from https://github.com/dotnet/coreclr/commit/322529912639e9ba0ea2306181b9f6034fdee5ae

diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs
index 0b258b7..e1bdcd8 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs
@@ -266,14 +266,18 @@ namespace System.Collections.ObjectModel
             {
                 ThrowHelper.IfNullAndNullsAreIllegalThenThrow<T>(value, ExceptionArgument.value);
 
+                T item = default(T)!;
+
                 try
                 {
-                    this[index] = (T)value!;
+                    item = (T)value!;
                 }
                 catch (InvalidCastException)
                 {
                     ThrowHelper.ThrowWrongValueTypeArgumentException(value, typeof(T));
                 }
+
+                this[index] = item;
             }
         }
 
@@ -309,15 +313,19 @@ namespace System.Collections.ObjectModel
             }
             ThrowHelper.IfNullAndNullsAreIllegalThenThrow<T>(value, ExceptionArgument.value);
 
+            T item = default(T)!;
+
             try
             {
-                Add((T)value!);
+                item = (T)value!;
             }
             catch (InvalidCastException)
             {
                 ThrowHelper.ThrowWrongValueTypeArgumentException(value, typeof(T));
             }
 
+            Add(item);
+
             return this.Count - 1;
         }
 
@@ -347,14 +355,18 @@ namespace System.Collections.ObjectModel
             }
             ThrowHelper.IfNullAndNullsAreIllegalThenThrow<T>(value, ExceptionArgument.value);
 
+            T item = default(T)!;
+
             try
             {
-                Insert(index, (T)value!);
+                item = (T)value!;
             }
             catch (InvalidCastException)
             {
                 ThrowHelper.ThrowWrongValueTypeArgumentException(value, typeof(T));
             }
+
+            Insert(index, item);
         }
 
         void IList.Remove(object? value)