Convert the encryption header parsers to be iterator-based.

components/gcm_driver/crypto/encryption_header_parsers_unittest.cc

Index: components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
diff --git a/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc b/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
index c8a1e0bfd1f804f3056716dcf14904bf008f64ff..d22c6b13cedafde2d0a88e78a98bd46032cdbb10 100644
--- a/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
+++ b/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
@@ -45,13 +45,16 @@ TEST(EncryptionHeaderParsersTest, ParseValidEncryptionHeaders) {
   for (size_t i = 0; i < arraysize(expected_results); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<EncryptionHeaderValues> values;
-    ASSERT_TRUE(ParseEncryptionHeader(expected_results[i].header, &values));
-    ASSERT_EQ(1u, values.size());
+    std::string header(expected_results[i].header);
 
-    EXPECT_EQ(expected_results[i].parsed_keyid, values[0].keyid);
-    EXPECT_EQ(expected_results[i].parsed_salt, values[0].salt);
-    EXPECT_EQ(expected_results[i].parsed_rs, values[0].rs);
+    EncryptionHeaderIterator iterator(header.begin(), header.end());
+    ASSERT_TRUE(iterator.GetNext());
+
+    EXPECT_EQ(expected_results[i].parsed_keyid, iterator.keyid());
+    EXPECT_EQ(expected_results[i].parsed_salt, iterator.salt());
+    EXPECT_EQ(expected_results[i].parsed_rs, iterator.rs());
+
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
@@ -84,15 +87,18 @@ TEST(EncryptionHeaderParsersTest, ParseValidMultiValueEncryptionHeaders) {
   for (size_t i = 0; i < arraysize(expected_results); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<EncryptionHeaderValues> values;
-    ASSERT_TRUE(ParseEncryptionHeader(expected_results[i].header, &values));
-    ASSERT_EQ(kNumberOfValues, values.size());
+    std::string header(expected_results[i].header);
 
+    EncryptionHeaderIterator iterator(header.begin(), header.end());
     for (size_t j = 0; j < kNumberOfValues; ++j) {
-      EXPECT_EQ(expected_results[i].parsed_values[j].keyid, values[j].keyid);
-      EXPECT_EQ(expected_results[i].parsed_values[j].salt, values[j].salt);
-      EXPECT_EQ(expected_results[i].parsed_values[j].rs, values[j].rs);
+      ASSERT_TRUE(iterator.GetNext());
+
+      EXPECT_EQ(expected_results[i].parsed_values[j].keyid, iterator.keyid());
+      EXPECT_EQ(expected_results[i].parsed_values[j].salt, iterator.salt());
+      EXPECT_EQ(expected_results[i].parsed_values[j].rs, iterator.rs());
     }
+
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
@@ -100,14 +106,13 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidEncryptionHeaders) {
   const char* const expected_failures[] = {
     "keyid",
     "keyid=",
-    "keyid=foo;novaluekey",
-    "keyid=foo,keyid",
+    "keyid=foo;keyid",
     "salt",
     "salt=",
     "salt=YmV/2ZXJ-sMDA",
     "salt=dHdlbHZlY29vbGJ5dGVz=====",
+    "salt=c2l4dGVlbmNvb2xieXRlcw;salt=123$xyz",
     "salt=123$xyz",
-    "salt=c2l4dGVlbmNvb2xieXRlcw,salt=123$xyz",
     "rs",
     "rs=",
     "rs=0",
@@ -116,16 +121,32 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidEncryptionHeaders) {
     "rs=-1",
     "rs=+5",
     "rs=99999999999999999999999999999999",
-    "rs=2,rs=0",
     "rs=foobar",
   };
 
+  const char* const expected_failures_second_iter[] = {
+    "keyid=foo,novaluekey",
+    "salt=c2l4dGVlbmNvb2xieXRlcw,salt=123$xyz",
+    "rs=2,rs=0",
+  };

[Ryan Sleevi, 2015/12/16 21:21:27]

As an aside (and perhaps not for this CL), the downside with a testing approach
like this is that it doesn't really clarify why things should fail; perhaps some
comments and grouping explanations may help (consider this more a remark than a
request for this CL; probably something as a cleanup CL later)

[Peter Beverloo, 2015/12/16 21:35:35]

Realistically, I'm unlikely to touch this code anytime soon, so better to do it
right now. Added comments and grouping where appropriate.

+
   for (size_t i = 0; i < arraysize(expected_failures); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<EncryptionHeaderValues> values;
-    EXPECT_FALSE(ParseEncryptionHeader(expected_failures[i], &values));
-    EXPECT_EQ(0u, values.size());
+    std::string header(expected_failures[i]);
+
+    EncryptionHeaderIterator iterator(header.begin(), header.end());
+    EXPECT_FALSE(iterator.GetNext());
+  }
+
+  for (size_t i = 0; i < arraysize(expected_failures_second_iter); i++) {
+    SCOPED_TRACE(i);
+
+    std::string header(expected_failures_second_iter[i]);
+
+    EncryptionHeaderIterator iterator(header.begin(), header.end());
+    EXPECT_TRUE(iterator.GetNext());
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
@@ -158,13 +179,16 @@ TEST(EncryptionHeaderParsersTest, ParseValidCryptoKeyHeaders) {
   for (size_t i = 0; i < arraysize(expected_results); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<CryptoKeyHeaderValues> values;
-    ASSERT_TRUE(ParseCryptoKeyHeader(expected_results[i].header, &values));
-    ASSERT_EQ(1u, values.size());
+    std::string header(expected_results[i].header);
+
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
+    ASSERT_TRUE(iterator.GetNext());
 
-    EXPECT_EQ(expected_results[i].parsed_keyid, values[0].keyid);
-    EXPECT_EQ(expected_results[i].parsed_aesgcm128, values[0].aesgcm128);
-    EXPECT_EQ(expected_results[i].parsed_dh, values[0].dh);
+    EXPECT_EQ(expected_results[i].parsed_keyid, iterator.keyid());
+    EXPECT_EQ(expected_results[i].parsed_aesgcm128, iterator.aesgcm128());
+    EXPECT_EQ(expected_results[i].parsed_dh, iterator.dh());
+
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
@@ -197,16 +221,19 @@ TEST(EncryptionHeaderParsersTest, ParseValidMultiValueCryptoKeyHeaders) {
   for (size_t i = 0; i < arraysize(expected_results); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<CryptoKeyHeaderValues> values;
-    ASSERT_TRUE(ParseCryptoKeyHeader(expected_results[i].header, &values));
-    ASSERT_EQ(kNumberOfValues, values.size());
+    std::string header(expected_results[i].header);
 
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
     for (size_t j = 0; j < kNumberOfValues; ++j) {
-      EXPECT_EQ(expected_results[i].parsed_values[j].keyid, values[j].keyid);
+      ASSERT_TRUE(iterator.GetNext());
+
+      EXPECT_EQ(expected_results[i].parsed_values[j].keyid, iterator.keyid());
       EXPECT_EQ(expected_results[i].parsed_values[j].aesgcm128,
-                values[j].aesgcm128);
-      EXPECT_EQ(expected_results[i].parsed_values[j].dh, values[j].dh);
+                iterator.aesgcm128());
+      EXPECT_EQ(expected_results[i].parsed_values[j].dh, iterator.dh());
     }
+
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
@@ -214,12 +241,11 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidCryptoKeyHeaders) {
   const char* const expected_failures[] = {
     "keyid",
     "keyid=",
-    "keyid=foo,keyid",
-    "keyid=foo;novaluekey",
+    "keyid=foo;keyid",
     "aesgcm128",
     "aesgcm128=",
     "aesgcm128=123$xyz",
-    "aesgcm128=foobar,aesgcm128=123$xyz",
+    "aesgcm128=foobar;aesgcm128=123$xyz",
     "dh",
     "dh=",
     "dh=YmV/2ZXJ-sMDA",
@@ -227,67 +253,92 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidCryptoKeyHeaders) {
     "dh=123$xyz",
   };
 
+  const char* const expected_failures_second_iter[] = {
+    "keyid=foo,novaluekey",
+    "dh=dHdlbHZlY29vbGJ5dGVz,aesgcm128=",
+  };
+
   for (size_t i = 0; i < arraysize(expected_failures); i++) {
     SCOPED_TRACE(i);
 
-    std::vector<CryptoKeyHeaderValues> values;
-    EXPECT_FALSE(ParseCryptoKeyHeader(expected_failures[i], &values));
-    EXPECT_EQ(0u, values.size());
+    std::string header(expected_failures[i]);
+
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
+    EXPECT_FALSE(iterator.GetNext());
   }
-}
 
-TEST(EncryptionHeaderParsersTest, SixValueHeader) {
-  const char* const header = "keyid=0,keyid=1,keyid=2,keyid=3,keyid=4,keyid=5";
+  for (size_t i = 0; i < arraysize(expected_failures_second_iter); i++) {
+    SCOPED_TRACE(i);
 
-  std::vector<EncryptionHeaderValues> encryption_values;
-  ASSERT_TRUE(ParseEncryptionHeader(header, &encryption_values));
+    std::string header(expected_failures_second_iter[i]);
+
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
+    EXPECT_TRUE(iterator.GetNext());
+    EXPECT_FALSE(iterator.GetNext());
+  }
+}
 
-  std::vector<CryptoKeyHeaderValues> crypto_key_values;
-  ASSERT_TRUE(ParseCryptoKeyHeader(header, &crypto_key_values));
+TEST(EncryptionHeaderParsersTest, SixValueHeader) {
+  const std::string header("keyid=0,keyid=1,keyid=2,keyid=3,keyid=4,keyid=5");
 
-  ASSERT_EQ(6u, encryption_values.size());
-  ASSERT_EQ(6u, crypto_key_values.size());
+  EncryptionHeaderIterator encryption_iterator(header.begin(), header.end());
+  CryptoKeyHeaderIterator crypto_key_iterator(header.begin(), header.end());
 
-  for (size_t i = 0; i < encryption_values.size(); i++) {
+  for (size_t i = 0; i < 6; ++i) {
     SCOPED_TRACE(i);
 
-    const std::string value = base::IntToString(i);
-
-    EXPECT_EQ(value, encryption_values[i].keyid);
-    EXPECT_EQ(value, crypto_key_values[i].keyid);
+    ASSERT_TRUE(encryption_iterator.GetNext());
+    ASSERT_TRUE(crypto_key_iterator.GetNext());
   }
+
+  EXPECT_FALSE(encryption_iterator.GetNext());
+  EXPECT_FALSE(crypto_key_iterator.GetNext());
 }
 
-TEST(EncryptionHeaderParsersTest, InvalidHeadersDoNotModifyOutput) {
-  EncryptionHeaderValues encryption_value;
-  encryption_value.keyid = "mykeyid";
-  encryption_value.salt = "somesalt";
-  encryption_value.rs = 42u;
+TEST(EncryptionHeaderParsersTest, InvalidHeadersResetOutput) {

[Ryan Sleevi, 2015/12/16 21:21:27]

Some comments here could help

[Peter Beverloo, 2015/12/16 21:35:35]

Done.

+  const std::string encryption_header(
+      "keyid=foo;salt=c2l4dGVlbmNvb2xieXRlcw;rs=1024,rs=foobar");
+  const std::string crypto_key_header(
+      "keyid=foo;aesgcm128=c2l4dGVlbmNvb2xieXRlcw;dh=dHdlbHZlY29vbGJ5dGVz,"
+      "aesgcm128=$$$");
+
+  EncryptionHeaderIterator encryption_iterator(
+      encryption_header.begin(), encryption_header.end());
+
+  ASSERT_EQ(0u, encryption_iterator.keyid().size());
+  ASSERT_EQ(0u, encryption_iterator.salt().size());
+  ASSERT_EQ(4096u, encryption_iterator.rs());
+
+  ASSERT_TRUE(encryption_iterator.GetNext());
+
+  EXPECT_EQ("foo", encryption_iterator.keyid());
+  EXPECT_EQ("sixteencoolbytes", encryption_iterator.salt());
+  EXPECT_EQ(1024u, encryption_iterator.rs());
+
+  ASSERT_FALSE(encryption_iterator.GetNext());
 
-  std::vector<EncryptionHeaderValues> encryption_values;
-  encryption_values.push_back(encryption_value);
+  EXPECT_EQ(0u, encryption_iterator.keyid().size());
+  EXPECT_EQ(0u, encryption_iterator.salt().size());
+  EXPECT_EQ(4096u, encryption_iterator.rs());
 
-  ASSERT_FALSE(ParseEncryptionHeader("rs=foobar", &encryption_values));
-  ASSERT_EQ(1u, encryption_values.size());
+  CryptoKeyHeaderIterator crypto_key_iterator(
+      crypto_key_header.begin(), crypto_key_header.end());
 
-  EXPECT_EQ("mykeyid", encryption_values[0].keyid);
-  EXPECT_EQ("somesalt", encryption_values[0].salt);
-  EXPECT_EQ(42u, encryption_values[0].rs);
+  ASSERT_EQ(0u, crypto_key_iterator.keyid().size());
+  ASSERT_EQ(0u, crypto_key_iterator.aesgcm128().size());
+  ASSERT_EQ(0u, crypto_key_iterator.dh().size());
 
-  CryptoKeyHeaderValues crypto_key_value;
-  crypto_key_value.keyid = "myotherkeyid";
-  crypto_key_value.aesgcm128 = "akey";
-  crypto_key_value.dh = "yourdh";
+  ASSERT_TRUE(crypto_key_iterator.GetNext());
 
-  std::vector<CryptoKeyHeaderValues> crypto_key_values;
-  crypto_key_values.push_back(crypto_key_value);
+  EXPECT_EQ("foo", crypto_key_iterator.keyid());
+  EXPECT_EQ("sixteencoolbytes", crypto_key_iterator.aesgcm128());
+  EXPECT_EQ("twelvecoolbytes", crypto_key_iterator.dh());
 
-  ASSERT_FALSE(ParseCryptoKeyHeader("aesgcm128=$$$", &crypto_key_values));
-  ASSERT_EQ(1u, crypto_key_values.size());
+  ASSERT_FALSE(crypto_key_iterator.GetNext());
 
-  EXPECT_EQ("myotherkeyid", crypto_key_values[0].keyid);
-  EXPECT_EQ("akey", crypto_key_values[0].aesgcm128);
-  EXPECT_EQ("yourdh", crypto_key_values[0].dh);
+  EXPECT_EQ(0u, crypto_key_iterator.keyid().size());
+  EXPECT_EQ(0u, crypto_key_iterator.aesgcm128().size());
+  EXPECT_EQ(0u, crypto_key_iterator.dh().size());
 }
 
 }  // namespace