Fix loading bad cookies from disk.

net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc

Index: net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
diff --git a/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc b/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
index d8be57ee0426f8334332d46231b0a625bce27494..4408e8dd4a6ce88b0297a56ef9e431dc98247331 100644
--- a/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
+++ b/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
@@ -480,7 +481,7 @@ TEST_F(SQLitePersistentCookieStoreTest, DISABLED_TestLoadOldSessionCookies) {
   cookies.clear();
 }
 
-// Test loading old session cookies from the disk.
+// Test refusing to load old session cookies from the disk.
 // TODO(mattcary): disabled for possibly causing iOS timeouts: crbug.com/727566.
 TEST_F(SQLitePersistentCookieStoreTest,
        DISABLED_TestDontLoadOldSessionCookies) {
@@ -511,6 +512,60 @@ TEST_F(SQLitePersistentCookieStoreTest,
   ASSERT_EQ(0U, cookies.size());
 }
 
+// Confirm bad cookies on disk don't get looaded
+TEST_F(SQLitePersistentCookieStoreTest, FilterBadCookies) {
+  // Create an on-disk store.
+  InitializeStore(false, true);
+  DestroyStore();
+
+  // Add some cookies in by hand.
+  base::FilePath store_name(temp_dir_.GetPath().Append(kCookieFilename));
+  std::unique_ptr<sql::Connection> db(base::MakeUnique<sql::Connection>());
+  ASSERT_TRUE(db->Open(store_name));
+  sql::Statement stmt(db->GetUniqueStatement(
+      "INSERT INTO cookies (creation_utc, host_key, name, value, "
+      "encrypted_value, path, expires_utc, secure, httponly, "
+      "firstpartyonly, last_access_utc, has_expires, persistent, priority) "
+      "VALUES (?,'google.izzle',?,?,'',?,0,0,0,0,0,1,1,0)"));
+  ASSERT_TRUE(stmt.is_valid());
+
+  struct CookieInfo {
+    const char* name;
+    const char* value;
+    const char* path;
+  } cookies_info[] = {// A couple non-canonical cookies.
+                      {"A=", "B", "/path"},
+                      {"C ", "D", "/path"},
+
+                      // A canonical cookie.
+                      {"E", "F", "/path"}};
+
+  int64_t creation_time = 1;
+  for (int i = 0;
+       static_cast<unsigned int>(i) < sizeof(cookies_info) / sizeof(CookieInfo);
+       i++) {

[mmenke, 2017/06/19 17:57:13]

for (const auto& cookie_info : cookies_info)

(Even without that  C++11 niftiness, there's also arraysize, which is also
prettier).

[Randy Smith (Not in Mondays), 2017/06/20 21:17:18]

Completely fair.  Done.

+    stmt.Reset(true);
+
+    stmt.BindInt64(0, creation_time++);
+    stmt.BindString(1, cookies_info[i].name);
+    stmt.BindString(2, cookies_info[i].value);
+    stmt.BindString(3, cookies_info[i].path);
+    ASSERT_TRUE(stmt.Run());
+  }
+  stmt.Clear();
+  db.reset();
+
+  // Reopen the store and confirm that the only cookie loaded is the
+  // canonical one.
+  CanonicalCookieVector cookies;
+  CreateAndLoad(false, false, &cookies);
+  ASSERT_EQ(1U, cookies.size());

[mmenke, 2017/06/19 17:57:13]

Hrm...Most of this file uses U, but there are a few cases of u.  I think u is
more common.  :(

[Randy Smith (Not in Mondays), 2017/06/20 21:17:18]

I'm not sure if you're requesting a change?  'u' is almost certainly more common
across the chromium codebase (haven't checked) but 'U' is more common in this
file (20 instances of 'U' versus 4 of 'u').  Given those two facts I'm inclined
to use U here (though I understand if that causes twitches).  Let me know if you
disagree.

+  EXPECT_STREQ("E", cookies[0]->Name().c_str());
+  EXPECT_STREQ("F", cookies[0]->Value().c_str());
+  EXPECT_STREQ("/path", cookies[0]->Path().c_str());
+  DestroyStore();
+}
+
 // TODO(mattcary): disabled for possibly causing iOS timeouts: crbug.com/727566.
 TEST_F(SQLitePersistentCookieStoreTest, DISABLED_PersistIsPersistent) {
   InitializeStore(false, true);