Merge pull request #2824 from justinvp/x509_collections

Cleanup and harden X509Certificate collections

Author: stephentoub

src/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Certificate2Collection.cs

@@ -1,14 +1,6 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
-using System.IO;
-using System.Text;
-using System.Diagnostics;
-using System.Globalization;
-using System.Runtime.InteropServices;
-
-using Internal.Cryptography;
 using Internal.Cryptography.Pal;
 
 namespace System.Security.Cryptography.X509Certificates
@@ -38,13 +30,11 @@ public X509Certificate2Collection(X509Certificate2Collection certificates)
         {
             get
             {
-                return (X509Certificate2)(List[index]);
+                return (X509Certificate2)(base[index]);
             }
             set
             {
-                if (value == null)
-                    throw new ArgumentNullException("value");
-                List[index] = value;
+                base[index] = value;
             }
         }
 
@@ -53,7 +43,7 @@ public int Add(X509Certificate2 certificate)
             if (certificate == null)
                 throw new ArgumentNullException("certificate");
 
-            return List.Add(certificate);
+            return base.Add(certificate);
         }
 
         public void AddRange(X509Certificate2[] certificates)
@@ -87,10 +77,9 @@ public void AddRange(X509Certificate2Collection certificates)
             int i = 0;
             try
             {
-                foreach (X509Certificate2 certificate in certificates)
+                for (; i < certificates.Count; i++)
                 {
-                    Add(certificate);
-                    i++;
+                    Add(certificates[i]);
                 }
             }
             catch
@@ -105,26 +94,27 @@ public void AddRange(X509Certificate2Collection certificates)
 
         public bool Contains(X509Certificate2 certificate)
         {
-            if (certificate == null)
-                throw new ArgumentNullException("certificate");
+            // This method used to throw ArgumentNullException, but it has been deliberately changed
+            // to no longer throw to match the behavior of X509CertificateCollection.Contains and the
+            // IList.Contains implementation, which do not throw.
 
-            return List.Contains(certificate);
+            return base.Contains(certificate);
         }
 
         public byte[] Export(X509ContentType contentType)
         {
             return Export(contentType, password: null);
         }
 
-        public byte[] Export(X509ContentType contentType, String password)
+        public byte[] Export(X509ContentType contentType, string password)
         {
             using (IStorePal storePal = StorePal.LinkFromCertificateCollection(this))
             {
                 return storePal.Export(contentType, password);
             }
         }
 
-        public X509Certificate2Collection Find(X509FindType findType, Object findValue, bool validOnly)
+        public X509Certificate2Collection Find(X509FindType findType, object findValue, bool validOnly)
         {
             if (findValue == null)
                 throw new ArgumentNullException("findValue");
@@ -139,16 +129,15 @@ public X509Certificate2Collection Find(X509FindType findType, Object findValue,
 
         public new X509Certificate2Enumerator GetEnumerator()
         {
-            X509CertificateEnumerator baseEnumerator = base.GetEnumerator();
-            return new X509Certificate2Enumerator(baseEnumerator);
+            return new X509Certificate2Enumerator(this);
         }
 
         public void Import(byte[] rawData)
         {
             Import(rawData, password: null, keyStorageFlags: X509KeyStorageFlags.DefaultKeySet);
         }
 
-        public void Import(byte[] rawData, String password, X509KeyStorageFlags keyStorageFlags)
+        public void Import(byte[] rawData, string password, X509KeyStorageFlags keyStorageFlags)
         {
             if (rawData == null)
                 throw new ArgumentNullException("rawData");
@@ -159,12 +148,12 @@ public void Import(byte[] rawData, String password, X509KeyStorageFlags keyStora
             }
         }
 
-        public void Import(String fileName)
+        public void Import(string fileName)
         {
             Import(fileName, password: null, keyStorageFlags: X509KeyStorageFlags.DefaultKeySet);
         }
 
-        public void Import(String fileName, String password, X509KeyStorageFlags keyStorageFlags)
+        public void Import(string fileName, string password, X509KeyStorageFlags keyStorageFlags)
         {
             if (fileName == null)
                 throw new ArgumentNullException("fileName");
@@ -180,15 +169,15 @@ public void Insert(int index, X509Certificate2 certificate)
             if (certificate == null)
                 throw new ArgumentNullException("certificate");
 
-            List.Insert(index, certificate);
+            base.Insert(index, certificate);
         }
 
         public void Remove(X509Certificate2 certificate)
         {
             if (certificate == null)
                 throw new ArgumentNullException("certificate");
 
-            List.Remove(certificate);
+            base.Remove(certificate);
         }
 
         public void RemoveRange(X509Certificate2[] certificates)
@@ -222,10 +211,9 @@ public void RemoveRange(X509Certificate2Collection certificates)
             int i = 0;
             try
             {
-                foreach (X509Certificate2 certificate in certificates)
+                for (; i < certificates.Count; i++)
                 {
-                    Remove(certificate);
-                    i++;
+                    Remove(certificates[i]);
                 }
             }
             catch
@@ -239,4 +227,3 @@ public void RemoveRange(X509Certificate2Collection certificates)
         }
     }
 }
-

src/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Certificate2Enumerator.cs

@@ -1,62 +1,56 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
-using System.IO;
-using System.Text;
 using System.Diagnostics;
 using System.Collections;
-using System.Globalization;
-using System.Runtime.InteropServices;
-
-using Internal.Cryptography;
+using System.Collections.Generic;
 
 namespace System.Security.Cryptography.X509Certificates
 {
     public sealed class X509Certificate2Enumerator : IEnumerator
     {
-        internal X509Certificate2Enumerator(IEnumerator baseEnumerator)
+        // This is a mutable struct enumerator, so don't mark it as readonly.
+        private List<X509Certificate>.Enumerator _enumerator;
+
+        internal X509Certificate2Enumerator(X509Certificate2Collection collection)
         {
-            _baseEnumerator = baseEnumerator;
+            Debug.Assert(collection != null);
+
+            collection.GetEnumerator(out _enumerator);
         }
 
         public X509Certificate2 Current
         {
-            get
-            {
-                return (X509Certificate2)(_baseEnumerator.Current);
-            }
+            // Call the struct enumerator's IEnumerator.Current implementation, which has the
+            // behavior we want of throwing InvalidOperationException when the enumerator
+            // hasn't been started or has ended, without boxing.
+            get { return (X509Certificate2)(EnumeratorHelper.GetCurrent(ref _enumerator)); }
         }
 
-        Object IEnumerator.Current
+        object IEnumerator.Current
         {
-            get
-            {
-                return _baseEnumerator.Current;
-            }
+            get { return Current; }
         }
 
         public bool MoveNext()
         {
-            return _baseEnumerator.MoveNext();
+            return _enumerator.MoveNext();
         }
 
         bool IEnumerator.MoveNext()
         {
-            return _baseEnumerator.MoveNext();
+            return MoveNext();
         }
 
         public void Reset()
         {
-            _baseEnumerator.Reset();
+            // Call Reset on the struct enumerator without boxing.
+            EnumeratorHelper.Reset(ref _enumerator);
         }
 
         void IEnumerator.Reset()
         {
-            _baseEnumerator.Reset();
+            Reset();
         }
-
-        private IEnumerator _baseEnumerator;
     }
 }
-