Add error details to ParseCertificate test data.

net/cert/internal/parse_certificate.cc

Index: net/cert/internal/parse_certificate.cc
diff --git a/net/cert/internal/parse_certificate.cc b/net/cert/internal/parse_certificate.cc
index af2f1c21dae8d8fc6d8dcb3733839cc3158e1cfb..2c42facf23e181d50f60bc8e3520d56617c6cb2b 100644
--- a/net/cert/internal/parse_certificate.cc
+++ b/net/cert/internal/parse_certificate.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/strings/string_util.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/der/input.h"
 #include "net/der/parse_values.h"
 #include "net/der/parser.h"
@@ -15,6 +16,20 @@ namespace net {
 
 namespace {
 
+DEFINE_CERT_ERROR_ID(kCertificateNotSequence,
+                     "Failed parsing Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kUnconsumedDataInsideCertificateSequence,
+                     "Unconsumed data inside Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kUnconsumedDataAfterCertificateSequence,
+                     "Unconsumed data after Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kTbsCertificateNotSequence,
+                     "Couldn't read Certificate as SEQUENCE");

[mattm, 2016/09/15 01:49:07]

TbsCertificate?

[eroman, 2016/09/16 01:24:56]

Done.

+DEFINE_CERT_ERROR_ID(
+    kSignatureAlgorithmNotSequence,
+    "Couldn't read Certificate.signatureAlgorithm as SEQUENCE");
+DEFINE_CERT_ERROR_ID(kSignatureValueNotBitString,
+                     "Couldn't read Certificate.signatureValue as BIT STRING");
+
 // Returns true if |input| is a SEQUENCE and nothing else.
 WARN_UNUSED_RESULT bool IsSequenceTLV(const der::Input& input) {
   der::Parser parser(input);
@@ -172,35 +187,54 @@ bool ParseCertificate(const der::Input& certificate_tlv,
                       der::Input* out_signature_algorithm_tlv,
                       der::BitString* out_signature_value,
                       CertErrors* out_errors) {
-  // TODO(crbug.com/634443): Fill |out_errors| (which may be null) with error
-  // information.
+  // |out_errors| is optional. But ensure it is non-null for the remainder of
+  // this function.
+  if (!out_errors) {
+    CertErrors unused_errors;
+    return ParseCertificate(certificate_tlv, out_tbs_certificate_tlv,
+                            out_signature_algorithm_tlv, out_signature_value,
+                            &unused_errors);
+  }
+
   der::Parser parser(certificate_tlv);
 
   //   Certificate  ::=  SEQUENCE  {
   der::Parser certificate_parser;
-  if (!parser.ReadSequence(&certificate_parser))
+  if (!parser.ReadSequence(&certificate_parser)) {
+    out_errors->AddError(kCertificateNotSequence);
     return false;
+  }
 
   //        tbsCertificate       TBSCertificate,
-  if (!ReadSequenceTLV(&certificate_parser, out_tbs_certificate_tlv))
+  if (!ReadSequenceTLV(&certificate_parser, out_tbs_certificate_tlv)) {
+    out_errors->AddError(kTbsCertificateNotSequence);
     return false;
+  }
 
   //        signatureAlgorithm   AlgorithmIdentifier,
-  if (!ReadSequenceTLV(&certificate_parser, out_signature_algorithm_tlv))
+  if (!ReadSequenceTLV(&certificate_parser, out_signature_algorithm_tlv)) {
+    out_errors->AddError(kSignatureAlgorithmNotSequence);
     return false;
+  }
 
   //        signatureValue       BIT STRING  }
-  if (!certificate_parser.ReadBitString(out_signature_value))
+  if (!certificate_parser.ReadBitString(out_signature_value)) {
+    out_errors->AddError(kSignatureValueNotBitString);
     return false;
+  }
 
   // There isn't an extension point at the end of Certificate.
-  if (certificate_parser.HasMore())
+  if (certificate_parser.HasMore()) {
+    out_errors->AddError(kUnconsumedDataInsideCertificateSequence);
     return false;
+  }
 
   // By definition the input was a single Certificate, so there shouldn't be
   // unconsumed data.
-  if (parser.HasMore())
+  if (parser.HasMore()) {
+    out_errors->AddError(kUnconsumedDataAfterCertificateSequence);
     return false;
+  }
 
   return true;
 }