From 84f70e5275adff514abedcf37799d7ffe09843b4 Mon Sep 17 00:00:00 2001
From: Jeffrey Zhao <jeffz@live.com>
Date: Thu, 8 Aug 2019 15:28:02 -0500
Subject: [PATCH] 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
---
 .../src/System/Collections/ObjectModel/Collection.cs   | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

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
--- 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)
-- 
2.7.4