SignedCertificateTimestamp storing & serialization code.

content/common/ssl_status_serialization.cc

Index: content/common/ssl_status_serialization.cc
diff --git a/content/common/ssl_status_serialization.cc b/content/common/ssl_status_serialization.cc
index 87593a595a2589e4b2d962dbb968b998e097af96..ab76931841b0840ad122316254c119c2e520839b 100644
--- a/content/common/ssl_status_serialization.cc
+++ b/content/common/ssl_status_serialization.cc
@@ -12,12 +12,21 @@ namespace content {
 std::string SerializeSecurityInfo(int cert_id,
                                   net::CertStatus cert_status,
                                   int security_bits,
-                                  int ssl_connection_status) {
+                                  int ssl_connection_status,
+                                  const SignedCertificateTimestampIDStatusList&
+                                      signed_certificate_timestamp_ids) {

[wtc, 2013/11/28 16:15:07]

Nit: We should reformat the parameter lists of these two functions as we did in
the header file.

[alcutter, 2013/11/28 16:42:18]

Done.

   Pickle pickle;
   pickle.WriteInt(cert_id);
   pickle.WriteUInt32(cert_status);
   pickle.WriteInt(security_bits);
   pickle.WriteInt(ssl_connection_status);
+  pickle.WriteInt(signed_certificate_timestamp_ids.size());
+  for (SignedCertificateTimestampIDStatusList::const_iterator iter =
+           signed_certificate_timestamp_ids.begin();
+       iter != signed_certificate_timestamp_ids.end(); ++iter) {
+    pickle.WriteInt(iter->id);
+    pickle.WriteUInt16(iter->status);
+  }
   return std::string(static_cast<const char*>(pickle.data()), pickle.size());
 }
 
@@ -25,8 +34,11 @@ bool DeserializeSecurityInfo(const std::string& state,
                              int* cert_id,
                              net::CertStatus* cert_status,
                              int* security_bits,
-                             int* ssl_connection_status) {
-  DCHECK(cert_id && cert_status && security_bits && ssl_connection_status);
+                             int* ssl_connection_status,
+                             SignedCertificateTimestampIDStatusList*
+                                 signed_certificate_timestamp_ids) {
+  DCHECK(cert_id && cert_status && security_bits && ssl_connection_status &&
+         signed_certificate_timestamp_ids);
   if (state.empty()) {
     // No SSL used.
     *cert_id = 0;
@@ -34,15 +46,34 @@ bool DeserializeSecurityInfo(const std::string& state,
     *cert_status = 0;
     *security_bits = -1;
     *ssl_connection_status = 0;
+    signed_certificate_timestamp_ids->clear();
     return false;
   }
 
   Pickle pickle(state.data(), static_cast<int>(state.size()));
   PickleIterator iter(pickle);
-  return pickle.ReadInt(&iter, cert_id) &&
-         pickle.ReadUInt32(&iter, cert_status) &&
-         pickle.ReadInt(&iter, security_bits) &&
-         pickle.ReadInt(&iter, ssl_connection_status);
+  bool pickle_read_ok = pickle.ReadInt(&iter, cert_id) &&
+                        pickle.ReadUInt32(&iter, cert_status) &&
+                        pickle.ReadInt(&iter, security_bits) &&
+                        pickle.ReadInt(&iter, ssl_connection_status);
+  if (!pickle_read_ok)
+    return pickle_read_ok;
+
+  int num_scts_to_read;
+  pickle_read_ok = pickle.ReadInt(&iter, &num_scts_to_read);
+  int id;
+  uint16 status;
+  for (; num_scts_to_read > 0 && pickle_read_ok; --num_scts_to_read) {

[wtc, 2013/11/28 16:15:07]

We should check pickle_read_ok before num_scts_to_read:
    pickle_read_ok && num_scts_to_read > 0

The reason is: if the pickle.ReadInt() call on line 63 fails, num_scts_to_read
may contain an uninitialized value, so we should read num_scts_to_read only if
pickle_read_ok is true.

[alcutter, 2013/11/28 16:42:18]

Yes, good catch.

+    pickle_read_ok &= pickle.ReadInt(&iter, &id) &&

[wtc, 2013/11/28 16:15:07]

Change "&=" to "=". If we get here, pickle_read_ok must be true because of the
for loop's condition.

[alcutter, 2013/11/28 16:42:18]

Done.

+                      pickle.ReadUInt16(&iter, &status);
+    if (pickle_read_ok)
+      signed_certificate_timestamp_ids->push_back(
+          SignedCertificateTimestampIDAndStatus(
+              id,
+              static_cast<net::ct::SCTVerifyStatus>(status)));

[wtc, 2013/11/28 16:15:07]

Nit: add curly braces around this if statement because it spans multiple lines.

Move this argument to the previous line.

[alcutter, 2013/11/28 16:42:18]

Done.

+  }
+
+  return pickle_read_ok;
 }
 
 }  // namespace content