commit 58a318edc892a595a5b043359a5d441869158699
author Bob Beck <bbe@google.com> Tue Feb 13 22:54:29 2024 +0000
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Feb 14 00:07:23 2024 +0000
tree e572178478cbc602ef10cb09a74d8edd650bb7f4
parent 80b08dfe5ef7d9190fa28d8204f2f7e8612134ee

Make ContainsError look only for Errors, not Warnings.

Currently the way this is used by code checking for errors
is to always assume that what it is looking for has actually
been added as an error. If one of them were added as
a warning, it would in theory do the wrong thing.

We can either delete the warnings, or just change ContainsError
to only consider errors to behave like the code is used today.

Change-Id: Id916281203122fffd1ffc323dc979ff4f8b6425b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66187
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Matt Mueller <mattm@google.com>

pki/cert_errors.cc

diff --git a/pki/cert_errors.cc b/pki/cert_errors.cc
index da95b49..3f5a77f 100644
--- a/pki/cert_errors.cc
+++ b/pki/cert_errors.cc
@@ -92,15 +92,20 @@
   return result;
 }
 
-bool CertErrors::ContainsError(CertErrorId id) const {
+bool CertErrors::ContainsErrorWithSeverity(CertErrorId id,
+                                           CertError::Severity severity) const {
   for (const CertError &node : nodes_) {
-    if (node.id == id) {
+    if (node.id == id && node.severity == severity) {
       return true;
     }
   }
   return false;
 }
 
+bool CertErrors::ContainsError(CertErrorId id) const {
+  return ContainsErrorWithSeverity(id, CertError::SEVERITY_HIGH);
+}
+
 bool CertErrors::ContainsAnyErrorWithSeverity(
     CertError::Severity severity) const {
   for (const CertError &node : nodes_) {

pki/cert_errors.h

diff --git a/pki/cert_errors.h b/pki/cert_errors.h
index 8f16e11..da35060 100644
--- a/pki/cert_errors.h
+++ b/pki/cert_errors.h
@@ -104,8 +104,13 @@
   // Dumps a textual representation of the errors for debugging purposes.
   std::string ToDebugString() const;
 
-  // Returns true if the error |id| was added to this CertErrors (of any
-  // severity).
+  // Returns true if the error |id| was added to this CertErrors at
+  // severity |severity|
+  bool ContainsErrorWithSeverity(CertErrorId id,
+                                 CertError::Severity severity) const;
+
+  // Returns true if the error |id| was added to this CertErrors at
+  // high serverity.
   bool ContainsError(CertErrorId id) const;
 
   // Returns true if this contains any errors of the given severity level.

pki/path_builder_unittest.cc

diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc
index 3bdeec9..624c0bc 100644
--- a/pki/path_builder_unittest.cc
+++ b/pki/path_builder_unittest.cc
@@ -2059,7 +2059,8 @@
   const CertErrors *cert1_errors =
       result.GetBestValidPath()->errors.GetErrorsForCert(1);
   ASSERT_TRUE(cert1_errors);
-  EXPECT_TRUE(cert1_errors->ContainsError(kWarningFromDelegate));
+  EXPECT_TRUE(cert1_errors->ContainsErrorWithSeverity(
+      kWarningFromDelegate, CertError::SEVERITY_WARNING));
 }
 
 DEFINE_CERT_ERROR_ID(kErrorFromDelegate, "Error from delegate");