Merge pull request #2835 from bartonjs/x509store_add_readonly

Fix behavior and test for X509Store.Add when store is readonly.

Author: bartonjs

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/DirectoryBasedStoreProvider.cs

@@ -190,6 +190,12 @@ private List<X509Certificate2> ReadDirectory()
 
         public void Add(ICertificatePal certPal)
         {
+            if (_readOnly)
+            {
+                // Windows compatibility: Remove only throws when it needs to do work, add throws always.
+                throw new CryptographicException(SR.Cryptography_X509_StoreReadOnly);
+            }
+
             // Save the collection to a local so it's consistent for the whole method
             List<X509Certificate2> certificates = _certificates;
             OpenSslX509CertificateReader cert = (OpenSslX509CertificateReader)certPal;
@@ -267,13 +273,6 @@ public void Add(ICertificatePal certPal)
                     }
                 }
 
-                if (_readOnly)
-                {
-                    // Windows compatibility: Don't throw until it has been established that
-                    // the certificate is not present, and we need to modify the filesystem.
-                    throw new CryptographicException(SR.Cryptography_X509_StoreReadOnly);
-                }
-
                 string destinationFilename;
                 FileMode mode = FileMode.CreateNew;
 

src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs

@@ -60,7 +60,7 @@ public static void AddReadOnlyThrows()
 
         [Fact]
         [ActiveIssue(2820, PlatformID.AnyUnix)]
-        public static void AddReadOnlyDoesntThrowWhenCertificateExists()
+        public static void AddReadOnlyThrowsWhenCertificateExists()
         {
             using (X509Store store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
             {
@@ -82,8 +82,7 @@ public static void AddReadOnlyDoesntThrowWhenCertificateExists()
 
                 if (toAdd != null)
                 {
-                    // This shouldn't throw, the cert is already present.
-                    store.Add(toAdd);
+                    Assert.Throws<CryptographicException>(() => store.Add(toAdd));
                 }
             }
         }