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

Commit 982d9d1

Browse files
committed
Cleanup X509Certificate collections
- Avoid enumerator allocations when enumerating the collections. - Reduce the allocations associated with creating an enumerator (use List<T>'s struct enumerator instead of a boxed IEnumerator). - Throw ArgumentNullException from Insert and Remove to match the full .NET Framework (desktop) behavior. Add tests to verify. - Remove some ArgumentOutOfRangeException checks as the underlying List<T> already throws that exception. Add tests to verify. - Use List<T>'s ICollection.SyncObject, allowing the field to be removed. - Remove unused usings. - Move fields to the top of class definitions. - Object -> object; String -> string. - Minor whitespace cleanup.
1 parent cda0ae1 commit 982d9d1

File tree

5 files changed

+269
-150
lines changed

5 files changed

+269
-150
lines changed

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
5-
using System.IO;
6-
using System.Text;
7-
using System.Diagnostics;
8-
using System.Globalization;
9-
using System.Runtime.InteropServices;
10-
11-
using Internal.Cryptography;
124
using Internal.Cryptography.Pal;
135

146
namespace System.Security.Cryptography.X509Certificates
@@ -44,6 +36,7 @@ public X509Certificate2Collection(X509Certificate2Collection certificates)
4436
{
4537
if (value == null)
4638
throw new ArgumentNullException("value");
39+
4740
List[index] = value;
4841
}
4942
}
@@ -87,10 +80,9 @@ public void AddRange(X509Certificate2Collection certificates)
8780
int i = 0;
8881
try
8982
{
90-
foreach (X509Certificate2 certificate in certificates)
83+
for (; i < certificates.Count; i++)
9184
{
92-
Add(certificate);
93-
i++;
85+
Add(certificates[i]);
9486
}
9587
}
9688
catch
@@ -116,15 +108,15 @@ public byte[] Export(X509ContentType contentType)
116108
return Export(contentType, password: null);
117109
}
118110

119-
public byte[] Export(X509ContentType contentType, String password)
111+
public byte[] Export(X509ContentType contentType, string password)
120112
{
121113
using (IStorePal storePal = StorePal.LinkFromCertificateCollection(this))
122114
{
123115
return storePal.Export(contentType, password);
124116
}
125117
}
126118

127-
public X509Certificate2Collection Find(X509FindType findType, Object findValue, bool validOnly)
119+
public X509Certificate2Collection Find(X509FindType findType, object findValue, bool validOnly)
128120
{
129121
if (findValue == null)
130122
throw new ArgumentNullException("findValue");
@@ -139,16 +131,15 @@ public X509Certificate2Collection Find(X509FindType findType, Object findValue,
139131

140132
public new X509Certificate2Enumerator GetEnumerator()
141133
{
142-
X509CertificateEnumerator baseEnumerator = base.GetEnumerator();
143-
return new X509Certificate2Enumerator(baseEnumerator);
134+
return new X509Certificate2Enumerator(this);
144135
}
145136

146137
public void Import(byte[] rawData)
147138
{
148139
Import(rawData, password: null, keyStorageFlags: X509KeyStorageFlags.DefaultKeySet);
149140
}
150141

151-
public void Import(byte[] rawData, String password, X509KeyStorageFlags keyStorageFlags)
142+
public void Import(byte[] rawData, string password, X509KeyStorageFlags keyStorageFlags)
152143
{
153144
if (rawData == null)
154145
throw new ArgumentNullException("rawData");
@@ -159,12 +150,12 @@ public void Import(byte[] rawData, String password, X509KeyStorageFlags keyStora
159150
}
160151
}
161152

162-
public void Import(String fileName)
153+
public void Import(string fileName)
163154
{
164155
Import(fileName, password: null, keyStorageFlags: X509KeyStorageFlags.DefaultKeySet);
165156
}
166157

167-
public void Import(String fileName, String password, X509KeyStorageFlags keyStorageFlags)
158+
public void Import(string fileName, string password, X509KeyStorageFlags keyStorageFlags)
168159
{
169160
if (fileName == null)
170161
throw new ArgumentNullException("fileName");
@@ -222,10 +213,9 @@ public void RemoveRange(X509Certificate2Collection certificates)
222213
int i = 0;
223214
try
224215
{
225-
foreach (X509Certificate2 certificate in certificates)
216+
for (; i < certificates.Count; i++)
226217
{
227-
Remove(certificate);
228-
i++;
218+
Remove(certificates[i]);
229219
}
230220
}
231221
catch
@@ -239,4 +229,3 @@ public void RemoveRange(X509Certificate2Collection certificates)
239229
}
240230
}
241231
}
242-
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,56 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
5-
using System.IO;
6-
using System.Text;
74
using System.Diagnostics;
85
using System.Collections;
9-
using System.Globalization;
10-
using System.Runtime.InteropServices;
11-
12-
using Internal.Cryptography;
6+
using System.Collections.Generic;
137

148
namespace System.Security.Cryptography.X509Certificates
159
{
1610
public sealed class X509Certificate2Enumerator : IEnumerator
1711
{
18-
internal X509Certificate2Enumerator(IEnumerator baseEnumerator)
12+
// This is a mutable struct enumerator, so don't mark it as readonly.
13+
private List<object>.Enumerator _enumerator;
14+
15+
internal X509Certificate2Enumerator(X509Certificate2Collection collection)
1916
{
20-
_baseEnumerator = baseEnumerator;
17+
Debug.Assert(collection != null);
18+
19+
collection.GetEnumerator(out _enumerator);
2120
}
2221

2322
public X509Certificate2 Current
2423
{
25-
get
26-
{
27-
return (X509Certificate2)(_baseEnumerator.Current);
28-
}
24+
// Call the struct enumerator's IEnumerator.Current implementation, which has the
25+
// behavior we want of throwing InvalidOperationException when the enumerator
26+
// hasn't been started or has ended, without boxing.
27+
get { return (X509Certificate2)(EnumeratorHelper.GetCurrent(ref _enumerator)); }
2928
}
3029

31-
Object IEnumerator.Current
30+
object IEnumerator.Current
3231
{
33-
get
34-
{
35-
return _baseEnumerator.Current;
36-
}
32+
get { return Current; }
3733
}
3834

3935
public bool MoveNext()
4036
{
41-
return _baseEnumerator.MoveNext();
37+
return _enumerator.MoveNext();
4238
}
4339

4440
bool IEnumerator.MoveNext()
4541
{
46-
return _baseEnumerator.MoveNext();
42+
return MoveNext();
4743
}
4844

4945
public void Reset()
5046
{
51-
_baseEnumerator.Reset();
47+
// Call Reset on the struct enumerator without boxing.
48+
EnumeratorHelper.Reset(ref _enumerator);
5249
}
5350

5451
void IEnumerator.Reset()
5552
{
56-
_baseEnumerator.Reset();
53+
Reset();
5754
}
58-
59-
private IEnumerator _baseEnumerator;
6055
}
6156
}
62-

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

Lines changed: 25 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,18 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
5-
using System.IO;
6-
using System.Text;
74
using System.Collections;
8-
using System.Diagnostics;
9-
using System.Globalization;
105
using System.Collections.Generic;
11-
using System.Runtime.InteropServices;
12-
using System.Threading;
13-
14-
using Interlocked = System.Threading.Interlocked;
15-
16-
using Internal.Cryptography;
176

187
namespace System.Security.Cryptography.X509Certificates
198
{
209
public partial class X509CertificateCollection : ICollection, IEnumerable, IList
2110
{
11+
private readonly List<object> _list;
12+
2213
public X509CertificateCollection()
2314
{
24-
_list = new List<Object>();
15+
_list = new List<object>();
2516
}
2617

2718
public X509CertificateCollection(X509Certificate[] value)
@@ -46,16 +37,9 @@ bool ICollection.IsSynchronized
4637
get { return false; }
4738
}
4839

49-
Object ICollection.SyncRoot
40+
object ICollection.SyncRoot
5041
{
51-
get
52-
{
53-
if (_syncRoot == null)
54-
{
55-
Interlocked.CompareExchange(ref _syncRoot, new object(), null);
56-
}
57-
return _syncRoot;
58-
}
42+
get { return ((ICollection)_list).SyncRoot; }
5943
}
6044

6145
bool IList.IsFixedSize
@@ -68,27 +52,21 @@ bool IList.IsReadOnly
6852
get { return false; }
6953
}
7054

71-
Object IList.this[int index]
55+
object IList.this[int index]
7256
{
7357
get
7458
{
75-
if (index < 0 || index >= Count)
76-
throw new ArgumentOutOfRangeException("index", SR.ArgumentOutOfRange_Index);
77-
7859
return _list[index];
7960
}
8061
set
8162
{
82-
if (index < 0 || index >= Count)
83-
throw new ArgumentOutOfRangeException("index", SR.ArgumentOutOfRange_Index);
8463
if (value == null)
8564
throw new ArgumentNullException("value");
8665

8766
_list[index] = value;
8867
}
8968
}
9069

91-
9270
public X509Certificate this[int index]
9371
{
9472
get
@@ -136,14 +114,7 @@ public void Clear()
136114

137115
public bool Contains(X509Certificate value)
138116
{
139-
foreach (X509Certificate cert in List)
140-
{
141-
if (cert.Equals(value))
142-
{
143-
return true;
144-
}
145-
}
146-
return false;
117+
return _list.Contains(value);
147118
}
148119

149120
public void CopyTo(X509Certificate[] array, int index)
@@ -153,18 +124,18 @@ public void CopyTo(X509Certificate[] array, int index)
153124

154125
public X509CertificateEnumerator GetEnumerator()
155126
{
156-
return new X509CertificateEnumerator(((IList<Object>)_list).GetEnumerator());
127+
return new X509CertificateEnumerator(this);
157128
}
158129

159130
IEnumerator IEnumerable.GetEnumerator()
160131
{
161-
return new X509CertificateEnumerator(((IList<Object>)_list).GetEnumerator());
132+
return GetEnumerator();
162133
}
163134

164135
public override int GetHashCode()
165136
{
166137
int hashCode = 0;
167-
foreach (X509Certificate cert in this)
138+
foreach (X509Certificate cert in _list)
168139
{
169140
hashCode += cert.GetHashCode();
170141
}
@@ -188,8 +159,6 @@ public void Remove(X509Certificate value)
188159

189160
public void RemoveAt(int index)
190161
{
191-
if (index < 0 || index >= Count)
192-
throw new ArgumentOutOfRangeException("index", SR.ArgumentOutOfRange_Index);
193162
_list.RemoveAt(index);
194163
}
195164

@@ -198,7 +167,7 @@ void ICollection.CopyTo(Array array, int index)
198167
((ICollection)_list).CopyTo(array, index);
199168
}
200169

201-
int IList.Add(Object value)
170+
int IList.Add(object value)
202171
{
203172
if (value == null)
204173
throw new ArgumentNullException("value");
@@ -208,23 +177,29 @@ int IList.Add(Object value)
208177
return index;
209178
}
210179

211-
bool IList.Contains(Object value)
180+
bool IList.Contains(object value)
212181
{
213182
return _list.Contains(value);
214183
}
215184

216-
int IList.IndexOf(Object value)
185+
int IList.IndexOf(object value)
217186
{
218187
return _list.IndexOf(value);
219188
}
220189

221-
void IList.Insert(int index, Object value)
190+
void IList.Insert(int index, object value)
222191
{
192+
if (value == null)
193+
throw new ArgumentNullException("value");
194+
223195
_list.Insert(index, value);
224196
}
225197

226-
void IList.Remove(Object value)
198+
void IList.Remove(object value)
227199
{
200+
if (value == null)
201+
throw new ArgumentNullException("value");
202+
228203
_list.Remove(value);
229204
}
230205

@@ -233,8 +208,9 @@ internal IList List
233208
get { return this; }
234209
}
235210

236-
private List<Object> _list;
237-
private Object _syncRoot;
211+
internal void GetEnumerator(out List<object>.Enumerator enumerator)
212+
{
213+
enumerator = _list.GetEnumerator();
214+
}
238215
}
239216
}
240-

0 commit comments

Comments
 (0)