Convert the encryption header parsers to be iterator-based.

components/gcm_driver/crypto/encryption_header_parsers_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/gcm_driver/crypto/encryption_header_parsers.h"
 
 #include <vector>
 
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 27 matching lines @@
     { "salt=c2l4dGVlbmNvb2xieXRlcw",
       "", "sixteencoolbytes", kDefaultRecordSize },
     { "rs=2048", "", "", 2048 },
     { "keyid=foo;someothervalue=1;rs=42", "foo", "", 42 },
     { "keyid=foo;keyid=bar", "bar", "", kDefaultRecordSize },
   };
 
   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());
   }
 }
 
 TEST(EncryptionHeaderParsersTest, ParseValidMultiValueEncryptionHeaders) {
   const size_t kNumberOfValues = 2u;
 
   struct {
     const char* const header;
     struct {
       const char* const keyid;
@@ skipping 12 matching lines @@
       { { "foo", "", kDefaultRecordSize },
         { "bar", "sixteencoolbytes", 1024 } } },
     { "keyid=\"foo,keyid=bar\",salt=c2l4dGVlbmNvb2xieXRlcw",
       { { "foo,keyid=bar", "", kDefaultRecordSize },
         { "", "sixteencoolbytes", kDefaultRecordSize } } },
   };
 
   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());
   }
 }
 
 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",
     "rs=0x13",
     "rs=1",
     "rs=-1",
     "rs=+5",
     "rs=99999999999999999999999999999999",
+    "rs=foobar",
+  };
+
+  const char* const expected_failures_second_iter[] = {
+    "keyid=foo,novaluekey",
+    "salt=c2l4dGVlbmNvb2xieXRlcw,salt=123$xyz",
     "rs=2,rs=0",
-    "rs=foobar",
   };

[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());
   }
 }
 
 TEST(EncryptionHeaderParsersTest, ParseValidCryptoKeyHeaders) {
   struct {
     const char* const header;
     const char* const parsed_keyid;
     const char* const parsed_aesgcm128;
     const char* const parsed_dh;
   } expected_results[] = {
@@ skipping 12 matching lines @@
     { "aesgcm128='c2l4dGVlbmNvb2xieXRlcw'", "", "sixteencoolbytes", "" },
     { "dh=dHdlbHZlY29vbGJ5dGVz", "", "", "twelvecoolbytes" },
     { "keyid=foo;someothervalue=bar;aesgcm128=dHdlbHZlY29vbGJ5dGVz",
       "foo", "twelvecoolbytes", "" },
     { "keyid=foo;keyid=bar", "bar", "", "" },
   };
 
   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);
 
-    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);
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
+    ASSERT_TRUE(iterator.GetNext());
+
+    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());
   }
 }
 
 TEST(EncryptionHeaderParsersTest, ParseValidMultiValueCryptoKeyHeaders) {
   const size_t kNumberOfValues = 2u;
 
   struct {
     const char* const header;
     struct {
       const char* const keyid;
@@ skipping 12 matching lines @@
       { { "foo", "", "" },
         { "bar", "", "twelvecoolbytes" } } },
     { "keyid=\"foo,keyid=bar\",aesgcm128=c2l4dGVlbmNvb2xieXRlcw",
       { { "foo,keyid=bar", "", "" },
         { "", "sixteencoolbytes", "" } } },
   };
 
   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());
   }
 }
 
 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",
     "dh=dHdlbHZlY29vbGJ5dGVz=====",
     "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());
+  }
+
+  for (size_t i = 0; i < arraysize(expected_failures_second_iter); i++) {
+    SCOPED_TRACE(i);
+
+    std::string header(expected_failures_second_iter[i]);
+
+    CryptoKeyHeaderIterator iterator(header.begin(), header.end());
+    EXPECT_TRUE(iterator.GetNext());
+    EXPECT_FALSE(iterator.GetNext());
   }
 }
 
 TEST(EncryptionHeaderParsersTest, SixValueHeader) {
-  const char* const header = "keyid=0,keyid=1,keyid=2,keyid=3,keyid=4,keyid=5";
+  const std::string header("keyid=0,keyid=1,keyid=2,keyid=3,keyid=4,keyid=5");
 
-  std::vector<EncryptionHeaderValues> encryption_values;
-  ASSERT_TRUE(ParseEncryptionHeader(header, &encryption_values));
+  EncryptionHeaderIterator encryption_iterator(header.begin(), header.end());
+  CryptoKeyHeaderIterator crypto_key_iterator(header.begin(), header.end());
 
-  std::vector<CryptoKeyHeaderValues> crypto_key_values;
-  ASSERT_TRUE(ParseCryptoKeyHeader(header, &crypto_key_values));
-
-  ASSERT_EQ(6u, encryption_values.size());
-  ASSERT_EQ(6u, crypto_key_values.size());
-
-  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);
+    ASSERT_TRUE(encryption_iterator.GetNext());
+    ASSERT_TRUE(crypto_key_iterator.GetNext());
+  }
 
-    EXPECT_EQ(value, encryption_values[i].keyid);
-    EXPECT_EQ(value, crypto_key_values[i].keyid);
-  }
+  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=$$$");
 
-  std::vector<EncryptionHeaderValues> encryption_values;
-  encryption_values.push_back(encryption_value);
+  EncryptionHeaderIterator encryption_iterator(
+      encryption_header.begin(), encryption_header.end());
 
-  ASSERT_FALSE(ParseEncryptionHeader("rs=foobar", &encryption_values));
-  ASSERT_EQ(1u, encryption_values.size());
+  ASSERT_EQ(0u, encryption_iterator.keyid().size());
+  ASSERT_EQ(0u, encryption_iterator.salt().size());
+  ASSERT_EQ(4096u, encryption_iterator.rs());
 
-  EXPECT_EQ("mykeyid", encryption_values[0].keyid);
-  EXPECT_EQ("somesalt", encryption_values[0].salt);
-  EXPECT_EQ(42u, encryption_values[0].rs);
+  ASSERT_TRUE(encryption_iterator.GetNext());
 
-  CryptoKeyHeaderValues crypto_key_value;
-  crypto_key_value.keyid = "myotherkeyid";
-  crypto_key_value.aesgcm128 = "akey";
-  crypto_key_value.dh = "yourdh";
+  EXPECT_EQ("foo", encryption_iterator.keyid());
+  EXPECT_EQ("sixteencoolbytes", encryption_iterator.salt());
+  EXPECT_EQ(1024u, encryption_iterator.rs());
 
-  std::vector<CryptoKeyHeaderValues> crypto_key_values;
-  crypto_key_values.push_back(crypto_key_value);
+  ASSERT_FALSE(encryption_iterator.GetNext());
 
-  ASSERT_FALSE(ParseCryptoKeyHeader("aesgcm128=$$$", &crypto_key_values));
-  ASSERT_EQ(1u, crypto_key_values.size());
+  EXPECT_EQ(0u, encryption_iterator.keyid().size());
+  EXPECT_EQ(0u, encryption_iterator.salt().size());
+  EXPECT_EQ(4096u, encryption_iterator.rs());
 
-  EXPECT_EQ("myotherkeyid", crypto_key_values[0].keyid);
-  EXPECT_EQ("akey", crypto_key_values[0].aesgcm128);
-  EXPECT_EQ("yourdh", crypto_key_values[0].dh);
+  CryptoKeyHeaderIterator crypto_key_iterator(
+      crypto_key_header.begin(), crypto_key_header.end());
+
+  ASSERT_EQ(0u, crypto_key_iterator.keyid().size());
+  ASSERT_EQ(0u, crypto_key_iterator.aesgcm128().size());
+  ASSERT_EQ(0u, crypto_key_iterator.dh().size());
+
+  ASSERT_TRUE(crypto_key_iterator.GetNext());
+
+  EXPECT_EQ("foo", crypto_key_iterator.keyid());
+  EXPECT_EQ("sixteencoolbytes", crypto_key_iterator.aesgcm128());
+  EXPECT_EQ("twelvecoolbytes", crypto_key_iterator.dh());
+
+  ASSERT_FALSE(crypto_key_iterator.GetNext());
+
+  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
 
 }  // namespace gcm