Prefer partial success to failure

If we read some certs from a file, but fail to read some from
a directory (or vv.) then return what we have.

This is good because it restores the crate's previous behaviour.
It also matches what (for example) golang crypto.x509 does.

This is bad because it hides a failure, which would be confusing
if a user relied on reading from _both_ a file and directory at
the same time.  Would someone do that?  The follow commit steps
towards ameliorating this, slightly.

Author: ctz

src/lib.rs

@@ -157,18 +157,33 @@ impl CertPaths {
             return Ok(None);
         }
 
+        let mut first_error = None;
+
         let mut certs = match &self.file {
-            Some(cert_file) => {
-                load_pem_certs(cert_file).map_err(|err| Self::load_err(cert_file, "file", err))?
-            }
+            Some(cert_file) => match load_pem_certs(cert_file)
+                .map_err(|err| Self::load_err(cert_file, "file", err))
+            {
+                Ok(certs) => certs,
+                Err(err) => {
+                    first_error = first_error.or(Some(err));
+                    Vec::new()
+                }
+            },
             None => Vec::new(),
         };
 
         if let Some(cert_dir) = &self.dir {
-            certs.append(
-                &mut load_pem_certs_from_dir(cert_dir)
-                    .map_err(|err| Self::load_err(cert_dir, "dir", err))?,
-            );
+            match load_pem_certs_from_dir(cert_dir)
+                .map_err(|err| Self::load_err(cert_dir, "dir", err))
+            {
+                Ok(mut from_dir) => certs.append(&mut from_dir),
+                Err(err) => first_error = first_error.or(Some(err)),
+            }
+        }
+
+        // promote first error if we have no certs to return
+        if let (Some(error), []) = (first_error, certs.as_slice()) {
+            return Err(error);
         }
 
         certs.sort_unstable_by(|a, b| a.cmp(b));

tests/smoketests.rs

@@ -184,3 +184,53 @@ fn badssl_with_dir_from_env() {
 
     check_site("self-signed.badssl.com").unwrap();
 }
+
+#[test]
+#[serial]
+#[ignore]
+#[cfg(target_os = "linux")]
+fn google_with_dir_but_broken_file() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
+
+    env::set_var("SSL_CERT_DIR", "/etc/ssl/certs");
+    env::set_var("SSL_CERT_FILE", "not-exist");
+    check_site("google.com").unwrap();
+}
+
+#[test]
+#[serial]
+#[ignore]
+#[cfg(target_os = "linux")]
+fn google_with_file_but_broken_dir() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
+
+    env::set_var("SSL_CERT_DIR", "/not-exist");
+    env::set_var("SSL_CERT_FILE", "/etc/ssl/certs/ca-certificates.crt");
+    check_site("google.com").unwrap();
+}
+
+#[test]
+#[serial]
+#[ignore]
+#[cfg(target_os = "linux")]
+fn nothing_works_with_broken_file_and_dir() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
+
+    env::set_var("SSL_CERT_DIR", "/not-exist");
+    env::set_var("SSL_CERT_FILE", "not-exist");
+    assert_eq!(
+        rustls_native_certs::load_native_certs()
+            .unwrap_err()
+            .to_string(),
+        "could not load certs from file not-exist: No such file or directory (os error 2)"
+    );
+}