Expose OpenSSL's key_exchange_info in the content API

content/common/ssl_status_serialization_unittest.cc

Index: content/common/ssl_status_serialization_unittest.cc
diff --git a/content/common/ssl_status_serialization_unittest.cc b/content/common/ssl_status_serialization_unittest.cc
index f8b4852f757440f270f3b685a326b679ac5ba7ef..72b3cb0c0216fecf232209c0fec7e65b006cf419 100644
--- a/content/common/ssl_status_serialization_unittest.cc
+++ b/content/common/ssl_status_serialization_unittest.cc
@@ -9,31 +9,44 @@
 
 namespace content {
 
+namespace {
+
+void InitializeSSLStatus(SSLStatus* status) {
+  status->security_style = SECURITY_STYLE_AUTHENTICATED;
+  status->cert_id = 1;
+  status->cert_status = net::CERT_STATUS_DATE_INVALID;
+  status->security_bits = 80;
+  status->key_exchange_info = 23;
+  status->connection_status = net::SSL_CONNECTION_VERSION_TLS1_2;
+  status->signed_certificate_timestamp_ids.push_back(
+      SignedCertificateTimestampIDAndStatus(1, net::ct::SCT_STATUS_OK));
+}
+
+void VerifyEqual(SSLStatus* const a, SSLStatus* const b) {

[Ryan Sleevi, 2015/09/02 01:37:07]

I'm not sure why you re-invent SSLStatus::Equals. Was it for the printing? If
so, just do this in the test:

void PrintTo(const SSLStatus& status, ::std::ostream* os) {
  *os << "Security Style: " << status.security_style;
  *os << "Cert ID: " << status.cert_id;
  ... etc
}

bool SSLStatusAreEqual(const SSLStatus& a, const SSLStatus &b) { return
a.Equals(b); }

EXPECT_PRED2(SSLStatusAreEqual, status, deserialized);

will do all the pretty-printing.

[Ryan Sleevi, 2015/09/02 01:37:07]

Both of these should be passed as const-ref, not constant pointers to
non-constant objects.

+  EXPECT_EQ(a->security_style, b->security_style);
+  EXPECT_EQ(a->cert_id, b->cert_id);
+  EXPECT_EQ(a->cert_status, b->cert_status);
+  EXPECT_EQ(a->security_bits, b->security_bits);
+  EXPECT_EQ(a->key_exchange_info, b->key_exchange_info);
+  EXPECT_EQ(a->connection_status, b->connection_status);
+  EXPECT_EQ(a->content_status, b->content_status);
+  EXPECT_EQ(a->signed_certificate_timestamp_ids.size(),
+            b->signed_certificate_timestamp_ids.size());
+}
+
+} // namespace
+
 // Test that a valid serialized SSLStatus returns true on
 // deserialization and deserializes correctly.
 TEST(SSLStatusSerializationTest, DeserializeSerializedStatus) {
   // Serialize dummy data and test that it deserializes properly.
-  SSLStatus status;
-  status.security_style = SECURITY_STYLE_AUTHENTICATED;
-  status.cert_id = 1;
-  status.cert_status = net::CERT_STATUS_DATE_INVALID;
-  status.security_bits = 80;
-  status.connection_status = net::SSL_CONNECTION_VERSION_TLS1_2;
-  SignedCertificateTimestampIDAndStatus sct(1, net::ct::SCT_STATUS_OK);
-  status.signed_certificate_timestamp_ids.push_back(sct);
-
+  SSLStatus status, deserialized;

[Ryan Sleevi, 2015/09/02 01:37:07]

Style wise, the original form was more consistent with the style guide

SSLStatus status;
InitializeDummySSLStatus(&status);
std::string serialized = SerializeSecurityInfo(status);

SSLStatus deserialized;
ASSERT_TRUE(DeserializeSecurityInfo(serialized, &deserialized));

[sigbjorn, 2015/09/02 13:42:14]

Done.

+  InitializeSSLStatus(&status);

[Ryan Sleevi, 2015/09/02 01:37:07]

Naming wise, this might be better renamed, as an SSLStatus (such as status or
deserialized) are already de-facto initialized.

[sigbjorn, 2015/09/02 13:42:14]

Done.

   std::string serialized = SerializeSecurityInfo(status);
-
-  SSLStatus deserialized;
   ASSERT_TRUE(DeserializeSecurityInfo(serialized, &deserialized));
-  EXPECT_EQ(status.security_style, deserialized.security_style);
-  EXPECT_EQ(status.cert_id, deserialized.cert_id);
-  EXPECT_EQ(status.cert_status, deserialized.cert_status);
-  EXPECT_EQ(status.security_bits, deserialized.security_bits);
-  EXPECT_EQ(status.connection_status, deserialized.connection_status);
-  EXPECT_EQ(status.signed_certificate_timestamp_ids.size(),
-            deserialized.signed_certificate_timestamp_ids.size());
-  EXPECT_EQ(sct, deserialized.signed_certificate_timestamp_ids[0]);
+  VerifyEqual(&status, &deserialized);
+  EXPECT_EQ(SignedCertificateTimestampIDAndStatus(1, net::ct::SCT_STATUS_OK),
+            deserialized.signed_certificate_timestamp_ids[0]);
   // Test that |content_status| has the default (initialized) value.
   EXPECT_EQ(SSLStatus::NORMAL_CONTENT, deserialized.content_status);
 }
@@ -43,69 +56,45 @@ TEST(SSLStatusSerializationTest, DeserializeSerializedStatus) {
 TEST(SSLStatusSerializationTest, DeserializeBogusStatus) {
   // Test that a failure to deserialize returns false and returns
   // initialized, default data.
-  SSLStatus invalid_deserialized;
+  SSLStatus default_ssl_status, invalid_deserialized;
   ASSERT_FALSE(
       DeserializeSecurityInfo("not an SSLStatus", &invalid_deserialized));
+  VerifyEqual(&default_ssl_status, &invalid_deserialized);

[Ryan Sleevi, 2015/09/02 01:37:07]

Use of a temp seems unnecessary

[sigbjorn, 2015/09/02 13:42:14]

Done.

+}
 
-  SSLStatus default_ssl_status;
-  EXPECT_EQ(default_ssl_status.security_style,
-            invalid_deserialized.security_style);
-  EXPECT_EQ(default_ssl_status.cert_id, invalid_deserialized.cert_id);
-  EXPECT_EQ(default_ssl_status.cert_status, invalid_deserialized.cert_status);
-  EXPECT_EQ(default_ssl_status.security_bits,
-            invalid_deserialized.security_bits);
-  EXPECT_EQ(default_ssl_status.connection_status,
-            invalid_deserialized.connection_status);
-  EXPECT_EQ(default_ssl_status.content_status,
-            invalid_deserialized.content_status);
-  EXPECT_EQ(0u, invalid_deserialized.signed_certificate_timestamp_ids.size());
-
-  // Serialize a status with a bad |security_bits| value and test that
-  // deserializing it fails.
-  SSLStatus status;
-  status.security_style = SECURITY_STYLE_AUTHENTICATED;
-  status.cert_id = 1;
-  status.cert_status = net::CERT_STATUS_DATE_INVALID;
+// Serialize a status with a bad |security_bits| value and test that
+// deserializing it fails.
+TEST(SSLStatusSerializationTest, DeserializeBogusSecurityBits) {
+  SSLStatus status, default_ssl_status, invalid_deserialized;
+  InitializeSSLStatus(&status);
   // |security_bits| must be <-1. (-1 means the strength is unknown, and
   // |0 means the connection is not encrypted).
   status.security_bits = -5;
-  status.connection_status = net::SSL_CONNECTION_VERSION_TLS1_2;
-  SignedCertificateTimestampIDAndStatus sct(1, net::ct::SCT_STATUS_OK);
-  status.signed_certificate_timestamp_ids.push_back(sct);
-
   std::string serialized = SerializeSecurityInfo(status);
   ASSERT_FALSE(DeserializeSecurityInfo(serialized, &invalid_deserialized));
+  VerifyEqual(&default_ssl_status, &invalid_deserialized);
+}
 
-  EXPECT_EQ(default_ssl_status.security_style,
-            invalid_deserialized.security_style);
-  EXPECT_EQ(default_ssl_status.cert_id, invalid_deserialized.cert_id);
-  EXPECT_EQ(default_ssl_status.cert_status, invalid_deserialized.cert_status);
-  EXPECT_EQ(default_ssl_status.security_bits,
-            invalid_deserialized.security_bits);
-  EXPECT_EQ(default_ssl_status.connection_status,
-            invalid_deserialized.connection_status);
-  EXPECT_EQ(default_ssl_status.content_status,
-            invalid_deserialized.content_status);
-  EXPECT_EQ(0u, invalid_deserialized.signed_certificate_timestamp_ids.size());
+// Serialize a status with a bad |key_exchange_info| value and test that
+// deserializing it fails.
+TEST(SSLStatusSerializationTest, DeserializeBogusKeyExchangeInfo) {
+  SSLStatus status, default_ssl_status, invalid_deserialized;
+  InitializeSSLStatus(&status);
+  status.key_exchange_info = -1;
+  std::string serialized = SerializeSecurityInfo(status);
+  ASSERT_FALSE(DeserializeSecurityInfo(serialized, &invalid_deserialized));
+  VerifyEqual(&default_ssl_status, &invalid_deserialized);
+}
 
-  // Now serialize a status with a bad |security_style| value and test
-  // that deserializing fails.
-  status.security_bits = 128;
+// Serialize a status with a bad |security_style| value and test that
+// deserializing it fails.
+TEST(SSLStatusSerializationTest, DeserializeBogusSecurityStyle) {
+  SSLStatus status, default_ssl_status, invalid_deserialized;

[Ryan Sleevi, 2015/09/02 01:37:07]

This sort of rewrite seems inconsistent with the style guide

SSLStatus status;
InitializeSSLStatus(&status);
...
std::string serialized = ...

SSLStatus invalid_deserialized;
ASSERT_FALSE(DeserializeSecurityInfo(...));

ASSERT_TRUE(SSLStatusAreEqual(SSLStatus(), invalid_deserialized));

[sigbjorn, 2015/09/02 13:42:14]

Done.

+  InitializeSSLStatus(&status);
   status.security_style = static_cast<SecurityStyle>(100);
-  serialized = SerializeSecurityInfo(status);
+  std::string serialized = SerializeSecurityInfo(status);
   ASSERT_FALSE(DeserializeSecurityInfo(serialized, &invalid_deserialized));
-
-  EXPECT_EQ(default_ssl_status.security_style,
-            invalid_deserialized.security_style);
-  EXPECT_EQ(default_ssl_status.cert_id, invalid_deserialized.cert_id);
-  EXPECT_EQ(default_ssl_status.cert_status, invalid_deserialized.cert_status);
-  EXPECT_EQ(default_ssl_status.security_bits,
-            invalid_deserialized.security_bits);
-  EXPECT_EQ(default_ssl_status.connection_status,
-            invalid_deserialized.connection_status);
-  EXPECT_EQ(default_ssl_status.content_status,
-            invalid_deserialized.content_status);
-  EXPECT_EQ(0u, invalid_deserialized.signed_certificate_timestamp_ids.size());
+  VerifyEqual(&default_ssl_status, &invalid_deserialized);
 }
 
 }  // namespace