From 6cbb5d491db7937031e0f33a9581ef49c05b3e5a Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 17 Jun 2016 09:49:47 -0700 Subject: [PATCH] Remove SafeX509Handle finalizations coming out of the test project There are a few classes of problems here * X509Chain's X509ChainElements have (newly created) X509Certificate2s on them, no one called Dispose. * Not Disposing the returned results from calls to Find. * Touching X509Store.Certificates in passing. * MachineRootStore_NonEmpty asked for the Count of the root store, sending that number of handles to be Finalized Commit migrated from https://github.com/dotnet/corefx/commit/67d802cebe2ca7ce6c5668907f382823758ad8f5 --- .../tests/ChainHolder.cs | 32 ++++ .../tests/ChainTests.cs | 52 +++--- .../tests/CollectionTests.cs | 8 +- .../tests/ExtensionsTests.cs | 15 +- .../tests/FindTests.cs | 181 +++++++++++++-------- ...rity.Cryptography.X509Certificates.Tests.csproj | 3 +- .../tests/X509FilesystemTests.Unix.cs | 46 +++--- .../tests/X509StoreTests.cs | 7 +- 8 files changed, 226 insertions(+), 118 deletions(-) create mode 100644 src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainHolder.cs diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainHolder.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainHolder.cs new file mode 100644 index 0000000..fd4096b --- /dev/null +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainHolder.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace System.Security.Cryptography.X509Certificates.Tests +{ + /// + /// A type to extend the Dispose on X509Chain to also dispose all of the X509Certificate objects + /// in the ChainElements structure. + /// + internal sealed class ChainHolder : IDisposable + { + public X509Chain Chain { get; } = new X509Chain(); + + public void Dispose() + { + DisposeChainElements(); + + Chain.Dispose(); + } + + public void DisposeChainElements() + { + int count = Chain.ChainElements.Count; + + for (int i = 0; i < count; i++) + { + Chain.ChainElements[i].Certificate.Dispose(); + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 2719c8a..ac611cd 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -4,8 +4,6 @@ using System.Collections.Generic; using System.IO; -using System.Linq; -using Microsoft.Win32.SafeHandles; using Xunit; namespace System.Security.Cryptography.X509Certificates.Tests @@ -19,8 +17,9 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) using (var unrelated = new X509Certificate2(TestData.DssCer)) + using (var chainHolder = new ChainHolder()) { - X509Chain chain = new X509Chain(); + X509Chain chain = chainHolder.Chain; chain.ChainPolicy.ExtraStore.Add(unrelated); chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); @@ -54,21 +53,20 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void BuildChainExtraStoreUntrustedRoot() { using (var testCert = new X509Certificate2(Path.Combine("TestData", "test.pfx"), TestData.ChainPfxPassword)) + using (ImportedCollection ic = Cert.Import(Path.Combine("TestData", "test.pfx"), TestData.ChainPfxPassword, X509KeyStorageFlags.DefaultKeySet)) + using (var chainHolder = new ChainHolder()) { - using (ImportedCollection ic = Cert.Import(Path.Combine("TestData", "test.pfx"), TestData.ChainPfxPassword, X509KeyStorageFlags.DefaultKeySet)) - { - X509Certificate2Collection collection = ic.Collection; + X509Certificate2Collection collection = ic.Collection; - X509Chain chain = new X509Chain(); - chain.ChainPolicy.ExtraStore.AddRange(collection); - chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.VerificationTime = new DateTime(2015, 9, 22, 12, 25, 0); + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.ExtraStore.AddRange(collection); + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.VerificationTime = new DateTime(2015, 9, 22, 12, 25, 0); - bool valid = chain.Build(testCert); + bool valid = chain.Build(testCert); - Assert.False(valid); - Assert.Contains(chain.ChainStatus, s => s.Status == X509ChainStatusFlags.UntrustedRoot); - } + Assert.False(valid); + Assert.Contains(chain.ChainStatus, s => s.Status == X509ChainStatusFlags.UntrustedRoot); } } @@ -136,8 +134,9 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) + using (var chainHolder = new ChainHolder()) { - X509Chain chain = new X509Chain(); + X509Chain chain = chainHolder.Chain; chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); @@ -163,8 +162,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void BuildChain_WithApplicationPolicy_Match() { using (var msCer = new X509Certificate2(TestData.MsCertificate)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + // Code Signing chain.ChainPolicy.ApplicationPolicy.Add(new Oid("1.3.6.1.5.5.7.3.3")); chain.ChainPolicy.VerificationTime = msCer.NotBefore.AddHours(2); @@ -182,8 +183,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void BuildChain_WithApplicationPolicy_NoMatch() { using (var cert = new X509Certificate2(TestData.MsCertificate)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + // Gibberish. (Code Signing + ".1") chain.ChainPolicy.ApplicationPolicy.Add(new Oid("1.3.6.1.5.5.7.3.3.1")); chain.ChainPolicy.VerificationFlags = @@ -210,8 +213,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void BuildChain_WithCertificatePolicy_Match() { using (var cert = new X509Certificate2(TestData.CertWithPolicies)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + // Code Signing chain.ChainPolicy.CertificatePolicy.Add(new Oid("2.18.19")); chain.ChainPolicy.VerificationFlags = @@ -229,8 +234,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void BuildChain_WithCertificatePolicy_NoMatch() { using (var cert = new X509Certificate2(TestData.CertWithPolicies)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.CertificatePolicy.Add(new Oid("2.999")); chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; @@ -257,9 +264,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests public static void VerifyWithRevocation() { using (var cert = new X509Certificate2(Path.Combine("TestData", "MS.cer"))) - using (var onlineChain = new X509Chain()) - using (var offlineChain = new X509Chain()) + using (var onlineChainHolder = new ChainHolder()) + using (var offlineChainHolder = new ChainHolder()) { + X509Chain onlineChain = onlineChainHolder.Chain; + X509Chain offlineChain = offlineChainHolder.Chain; + onlineChain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs index bc45ae5..bf49246 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs @@ -865,8 +865,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; @@ -1272,8 +1274,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) - using (X509Chain chain = new X509Chain()) + using (var chainHolder = new ChainHolder()) { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExtensionsTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExtensionsTests.cs index a701faf..0559f1f 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExtensionsTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExtensionsTests.cs @@ -344,7 +344,13 @@ namespace System.Security.Cryptography.X509Certificates.Tests [Fact] public static void SubjectKeyIdentifierExtension_PublicKey() { - PublicKey pk = new X509Certificate2(TestData.MsCertificate).PublicKey; + PublicKey pk; + + using (var cert = new X509Certificate2(TestData.MsCertificate)) + { + pk = cert.PublicKey; + } + X509SubjectKeyIdentifierExtension e = new X509SubjectKeyIdentifierExtension(pk, false); byte[] rawData = e.RawData; @@ -438,7 +444,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests byte[] expectedDer, string expectedIdentifier) { - PublicKey pk = new X509Certificate2(certBytes).PublicKey; + PublicKey pk; + + using (var cert = new X509Certificate2(certBytes)) + { + pk = cert.PublicKey; + } X509SubjectKeyIdentifierExtension ext = new X509SubjectKeyIdentifierExtension(pk, algorithm, critical); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/FindTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/FindTests.cs index 316c60a..ecb5c55 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/FindTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/FindTests.cs @@ -47,7 +47,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests { X509Certificate2Collection col2 = col1.Find(findType, findValue, validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -78,28 +81,31 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection col2 = col1.Find(findType, findValue, validOnly: false); - Assert.Equal(1, col2.Count); - - byte[] serialNumber; - - using (X509Certificate2 match = col2[0]) + using (new ImportedCollection(col2)) { - Assert.Equal(expected, match); - Assert.NotSame(expected, match); + Assert.Equal(1, col2.Count); - // FriendlyName is Windows-only. - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + byte[] serialNumber; + + using (X509Certificate2 match = col2[0]) { - // Verify that the find result and original are linked, not just equal. - match.FriendlyName = "HAHA"; - Assert.Equal("HAHA", expected.FriendlyName); + Assert.Equal(expected, match); + Assert.NotSame(expected, match); + + // FriendlyName is Windows-only. + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // Verify that the find result and original are linked, not just equal. + match.FriendlyName = "HAHA"; + Assert.Equal("HAHA", expected.FriendlyName); + } + + serialNumber = match.GetSerialNumber(); } - serialNumber = match.GetSerialNumber(); + // Check that disposing match didn't dispose expected + Assert.Equal(serialNumber, expected.GetSerialNumber()); } - - // Check that disposing match didn't dispose expected - Assert.Equal(serialNumber, expected.GetSerialNumber()); } [Theory] @@ -183,25 +189,28 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection col2 = col1.Find(X509FindType.FindByThumbprint, msCer.Thumbprint, validOnly); - // The certificate is expired. Unless we invent time travel - // (or roll the computer clock back significantly), the validOnly - // criteria should filter it out. - // - // This test runs both ways to make sure that the precondition of - // "would match otherwise" is met (in the validOnly=false case it is - // effectively the same as FindByValidThumbprint, but does less inspection) - int expectedMatchCount = validOnly ? 0 : 1; - - Assert.Equal(expectedMatchCount, col2.Count); - - if (!validOnly) + using (new ImportedCollection(col2)) { - // Double check that turning on validOnly doesn't prevent the cloning - // behavior of Find. - using (X509Certificate2 match = col2[0]) + // The certificate is expired. Unless we invent time travel + // (or roll the computer clock back significantly), the validOnly + // criteria should filter it out. + // + // This test runs both ways to make sure that the precondition of + // "would match otherwise" is met (in the validOnly=false case it is + // effectively the same as FindByValidThumbprint, but does less inspection) + int expectedMatchCount = validOnly ? 0 : 1; + + Assert.Equal(expectedMatchCount, col2.Count); + + if (!validOnly) { - Assert.Equal(msCer, match); - Assert.NotSame(msCer, match); + // Double check that turning on validOnly doesn't prevent the cloning + // behavior of Find. + using (X509Certificate2 match = col2[0]) + { + Assert.Equal(msCer, match); + Assert.NotSame(msCer, match); + } } } } @@ -307,7 +316,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests earliest - TimeSpan.FromSeconds(1), validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -324,7 +336,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests latest + TimeSpan.FromSeconds(1), validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -350,7 +365,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests noMatchTime, validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -391,7 +409,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests matchTime, validOnly: false); - Assert.Equal(1, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(1, col2.Count); + } }); } @@ -418,7 +439,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests noMatchTime, validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -445,7 +469,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests matchTime, validOnly: false); - Assert.Equal(1, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(1, col2.Count); + } }); } @@ -472,7 +499,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests noMatchTime, validOnly: false); - Assert.Equal(0, col2.Count); + using (new ImportedCollection(col2)) + { + Assert.Equal(0, col2.Count); + } }); } @@ -648,14 +678,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection results = col1.Find(X509FindType.FindByApplicationPolicy, "1.3.6.1.5.5.7.3.3", false); - Assert.Equal(2, results.Count); - - Assert.True(results.Contains(msCer)); - Assert.True(results.Contains(pfxCer)); - - foreach (X509Certificate2 match in results) + using (new ImportedCollection(results)) { - match.Dispose(); + Assert.Equal(2, results.Count); + + Assert.True(results.Contains(msCer)); + Assert.True(results.Contains(pfxCer)); } }); } @@ -679,7 +707,10 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection results = col1.Find(X509FindType.FindByApplicationPolicy, "2.2", false); - Assert.Equal(0, results.Count); + using (new ImportedCollection(results)) + { + Assert.Equal(0, results.Count); + } }); } @@ -717,7 +748,11 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection col1 = new X509Certificate2Collection(policyCert); X509Certificate2Collection results = col1.Find(X509FindType.FindByCertificatePolicy, "2.999", false); - Assert.Equal(0, results.Count); + + using (new ImportedCollection(results)) + { + Assert.Equal(0, results.Count); + } } } @@ -755,7 +790,11 @@ namespace System.Security.Cryptography.X509Certificates.Tests X509Certificate2Collection col1 = new X509Certificate2Collection(templatedCert); X509Certificate2Collection results = col1.Find(X509FindType.FindByTemplateName, "2.999", false); - Assert.Equal(0, results.Count); + + using (new ImportedCollection(results)) + { + Assert.Equal(0, results.Count); + } } } @@ -790,29 +829,35 @@ namespace System.Security.Cryptography.X509Certificates.Tests var coll = new X509Certificate2Collection { noKeyUsages, noKeyUsages2, keyUsages, }; X509Certificate2Collection results = coll.Find(X509FindType.FindByKeyUsage, matchCriteria, false); - // The two certificates with no KeyUsages extension will always match, the real question is about the third. - int matchCount = shouldMatch ? 3 : 2; - Assert.Equal(matchCount, results.Count); - - if (shouldMatch) + using (new ImportedCollection(results)) { - bool found = false; + // The two certificates with no KeyUsages extension will always match, + // the real question is about the third. + int matchCount = shouldMatch ? 3 : 2; + Assert.Equal(matchCount, results.Count); - foreach (X509Certificate2 cert in results) + if (shouldMatch) { - if (keyUsages.Equals(cert)) + bool found = false; + + foreach (X509Certificate2 cert in results) { - Assert.NotSame(cert, keyUsages); - found = true; - break; + if (keyUsages.Equals(cert)) + { + Assert.NotSame(cert, keyUsages); + found = true; + break; + } } - } - Assert.True(found, "Certificate with key usages was found in the collection"); - } - else - { - Assert.False(results.Contains(keyUsages), "KeyUsages certificate is not present in the collection"); + Assert.True(found, "Certificate with key usages was found in the collection"); + } + else + { + Assert.False( + results.Contains(keyUsages), + "KeyUsages certificate is not present in the collection"); + } } } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index e06febf..d1a54ec 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -1,4 +1,4 @@ - + Windows_Debug @@ -26,6 +26,7 @@ + diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs index ce46f7b..9e27cfa 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509FilesystemTests.Unix.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; @@ -32,8 +31,9 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) using (var unrelated = new X509Certificate2(TestData.DssCer)) + using (var chainHolder = new ChainHolder()) { - X509Chain chain = new X509Chain(); + X509Chain chain = chainHolder.Chain; chain.ChainPolicy.ExtraStore.Add(unrelated); chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); @@ -55,6 +55,8 @@ namespace System.Security.Cryptography.X509Certificates.Tests Assert.Equal(X509ChainStatusFlags.UntrustedRoot, chain.ChainStatus[0].Status); } + chainHolder.DisposeChainElements(); + chain.ChainPolicy.RevocationMode = X509RevocationMode.Offline; valid = chain.Build(microsoftDotComIssuer); @@ -65,6 +67,8 @@ namespace System.Security.Cryptography.X509Certificates.Tests File.WriteAllText(crlFile, MicrosoftDotComRootCrlPem, Encoding.ASCII); + chainHolder.DisposeChainElements(); + valid = chain.Build(microsoftDotComIssuer); Assert.True(valid, "Chain should build validly now"); Assert.Equal(initialErrorCount, chain.ChainStatus.Length); @@ -550,38 +554,36 @@ namespace System.Security.Cryptography.X509Certificates.Tests using (X509Store storeClone = new X509Store(newName, store.Location)) { storeClone.Open(OpenFlags.ReadWrite); - - X509Certificate2Collection storeCerts = store.Certificates; - X509Certificate2Collection storeCloneCerts = storeClone.Certificates; - - Assert.Equal( - storeCerts.OfType(), - storeCloneCerts.OfType()); + AssertEqualContents(store, storeClone); store.Add(certC); // The object was added to store, but should show up in both objects // after re-reading the Certificates property - storeCerts = store.Certificates; - storeCloneCerts = storeClone.Certificates; - - Assert.Equal( - storeCerts.OfType(), - storeCloneCerts.OfType()); + AssertEqualContents(store, storeClone); // Now add one to storeClone to prove bidirectionality. storeClone.Add(certD); - storeCerts = store.Certificates; - storeCloneCerts = storeClone.Certificates; - - Assert.Equal( - storeCerts.OfType(), - storeCloneCerts.OfType()); + AssertEqualContents(store, storeClone); } } }); } - + + private static void AssertEqualContents(X509Store storeA, X509Store storeB) + { + Assert.NotSame(storeA, storeB); + + using (var storeATracker = new ImportedCollection(storeA.Certificates)) + using (var storeBTracker = new ImportedCollection(storeB.Certificates)) + { + X509Certificate2Collection storeACerts = storeATracker.Collection; + X509Certificate2Collection storeBCerts = storeBTracker.Collection; + + Assert.Equal(storeACerts.OfType(), storeBCerts.OfType()); + } + } + private static void RunX509StoreTest(Action testAction) { string certStoresFeaturePath = PersistedFiles.GetUserFeatureDirectory("cryptography", "x509stores"); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs index 72876af..b0cda7a 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs @@ -198,8 +198,11 @@ namespace System.Security.Cryptography.X509Certificates.Tests { store.Open(OpenFlags.ReadOnly); - int certCount = store.Certificates.Count; - Assert.InRange(certCount, MinimumThreshold, int.MaxValue); + using (var storeCerts = new ImportedCollection(store.Certificates)) + { + int certCount = storeCerts.Collection.Count; + Assert.InRange(certCount, MinimumThreshold, int.MaxValue); + } } } } -- 2.7.4