Don't force-allocate x509ChainPolicy collections in X509Chain.Build (dotnet/corefx...
authorStephen Toub <stoub@microsoft.com>
Sun, 10 Feb 2019 02:26:46 +0000 (21:26 -0500)
committerGitHub <noreply@github.com>
Sun, 10 Feb 2019 02:26:46 +0000 (21:26 -0500)
Previously X509ChainPolicy would always allocate its collections, but now it lazily allocates them.  However, X509Chain.Build is forcing them to be allocated even when they're not needed.  Stop doing that.

Commit migrated from https://github.com/dotnet/corefx/commit/5b8fe73971fb7885fa7afc08ceec28df9a695a23

src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/ChainPal.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509ChainProcessor.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Chain.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509ChainPolicy.cs

index 943fda15cb77e2811cf66d5c30fcfe42a384ffc2..5bf3d804d9a9a8faadf819381f461e048771f0ad 100644 (file)
@@ -100,21 +100,32 @@ namespace Internal.Cryptography.Pal
                 systemTrusted.DisposeAll();
                 downloaded.DisposeAll();
 
-                // Candidate certs which came from extraStore should NOT be disposed, since they came
-                // from outside.
-                var extraStoreByReference = new HashSet<X509Certificate2>(
-                    ReferenceEqualityComparer<X509Certificate2>.Instance);
-
-                foreach (X509Certificate2 extraCert in extraStore)
+                if (extraStore == null || extraStore.Count == 0)
                 {
-                    extraStoreByReference.Add(extraCert);
+                    // There were no extraStore certs, so everything can be disposed.
+                    foreach (X509Certificate2 candidate in candidates)
+                    {
+                        candidate.Dispose();
+                    }
                 }
-
-                foreach (X509Certificate2 candidate in candidates)
+                else
                 {
-                    if (!extraStoreByReference.Contains(candidate))
+                    // Candidate certs which came from extraStore should NOT be disposed, since they came
+                    // from outside.
+                    var extraStoreByReference = new HashSet<X509Certificate2>(
+                        ReferenceEqualityComparer<X509Certificate2>.Instance);
+
+                    foreach (X509Certificate2 extraCert in extraStore)
                     {
-                        candidate.Dispose();
+                        extraStoreByReference.Add(extraCert);
+                    }
+
+                    foreach (X509Certificate2 candidate in candidates)
+                    {
+                        if (!extraStoreByReference.Contains(candidate))
+                        {
+                            candidate.Dispose();
+                        }
                     }
                 }
 
index 47f8301f20a9cca085b4d4bc044a744b04cf8a9f..03406d1d2f0e95cb61bb375d081a2535cd7ee97d 100644 (file)
@@ -418,15 +418,30 @@ namespace Internal.Cryptography.Pal
                     }
                 }
 
-                X509Certificate2Collection[] storesToCheck =
+                X509Certificate2Collection[] storesToCheck;
+                if (extraStore != null && extraStore.Count > 0)
                 {
-                    extraStore,
-                    userMyCerts,
-                    userIntermediateCerts,
-                    systemIntermediateCerts,
-                    userRootCerts,
-                    systemRootCerts,
-                };
+                    storesToCheck = new[]
+                    {
+                        extraStore,
+                        userMyCerts,
+                        userIntermediateCerts,
+                        systemIntermediateCerts,
+                        userRootCerts,
+                        systemRootCerts,
+                    };
+                }
+                else
+                {
+                    storesToCheck = new[]
+                    {
+                        userMyCerts,
+                        userIntermediateCerts,
+                        systemIntermediateCerts,
+                        userRootCerts,
+                        systemRootCerts,
+                    };
+                }
 
                 while (toProcess.Count > 0)
                 {
index ae9a27902c6ba706d552a238ceca28aefb98a5fb..90309909b1e86b9bc7d39e1850872f6e1224607b 100644 (file)
@@ -118,9 +118,9 @@ namespace System.Security.Cryptography.X509Certificates
                 _pal = ChainPal.BuildChain(
                     _useMachineContext,
                     certificate.Pal,
-                    chainPolicy.ExtraStore,
-                    chainPolicy.ApplicationPolicy,
-                    chainPolicy.CertificatePolicy,
+                    chainPolicy._extraStore,
+                    chainPolicy._applicationPolicy,
+                    chainPolicy._certificatePolicy,
                     chainPolicy.RevocationMode,
                     chainPolicy.RevocationFlag,
                     chainPolicy.VerificationTime,
index 7bd0f3498c677327f31a99039dfa6810078f6bab..1270944b1e9f489315c9d2da5b07c45c5551110a 100644 (file)
@@ -11,9 +11,9 @@ namespace System.Security.Cryptography.X509Certificates
         private X509RevocationMode _revocationMode;
         private X509RevocationFlag _revocationFlag;
         private X509VerificationFlags _verificationFlags;
-        private OidCollection _applicationPolicy;
-        private OidCollection _certificatePolicy;
-        private X509Certificate2Collection _extraStore;
+        internal OidCollection _applicationPolicy;
+        internal OidCollection _certificatePolicy;
+        internal X509Certificate2Collection _extraStore;
 
         public X509ChainPolicy()
         {