If the version field is omitted from the certificate, the default

chrome/common/net/x509_certificate_model_nss.cc

Index: chrome/common/net/x509_certificate_model_nss.cc
===================================================================
--- chrome/common/net/x509_certificate_model_nss.cc	(revision 137201)
+++ chrome/common/net/x509_certificate_model_nss.cc	(working copy)
@@ -125,10 +125,13 @@
 }
 
 string GetVersion(X509Certificate::OSCertHandle cert_handle) {
-  unsigned long version = ULONG_MAX;
-  if (SEC_ASN1DecodeInteger(&cert_handle->version, &version) == SECSuccess &&
-      version != ULONG_MAX)

[wtc, 2012/05/18 01:15:57]

I guess the version != ULONG_MAX check here is to work
around a bug in SEC_ASN1DecodeInteger when
    cert_handle->version.data != NULL
    cert_handle->version,len == 0

In that case, SEC_ASN1DecodeInteger returns SECSuccess
and stores either 0 or (unsigned long)-1 in the output
argument.

In the new code, we don't call SEC_ASN1DecodeInteger
if cert_handle->version.len == 0, so this workaround is
not necessary.

[mattm, 2012/05/18 03:19:50]

Honestly I can't remember why it's that way, but that sounds likely.
sounds good.  Do we need to check version.data == NULL also or will len always
be 0 in that case?

[wtc, 2012/05/18 21:47:13]

In my testing with several recent versions of NSS, I always
see data == NULL, len == 0.  So it should be sufficient to
just check version.len == 0.

+  // If the version field is omitted from the certificate, the default
+  // value is v1(0).
+  unsigned long version = 0;
+  if (cert_handle->version.len == 0 ||
+      SEC_ASN1DecodeInteger(&cert_handle->version, &version) == SECSuccess) {
     return base::UintToString(version + 1);
+  }
   return "";
 }