Fix loading bad cookies from disk.

net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc

 // Copyright (c) 2012 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 "net/extras/sqlite/sqlite_persistent_cookie_store.h"
 
 #include <map>
 #include <memory>
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #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"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/time/time.h"
 #include "crypto/encryptor.h"
 #include "crypto/symmetric_key.h"
 #include "net/cookies/canonical_cookie.h"
@@ skipping 446 matching lines @@
 
   ASSERT_EQ(1U, cookies.size());
   ASSERT_STREQ("sessioncookie.com", cookies[0]->Domain().c_str());
   ASSERT_STREQ("C", cookies[0]->Name().c_str());
   ASSERT_STREQ("D", cookies[0]->Value().c_str());
   ASSERT_EQ(COOKIE_PRIORITY_DEFAULT, cookies[0]->Priority());
 
   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) {
   InitializeStore(false, true);
 
   // Add a session cookie.
   store_->AddCookie(
       CanonicalCookie("C", "D", "sessioncookie.com", "/", base::Time::Now(),
                       base::Time(), base::Time(), false, false,
                       CookieSameSite::DEFAULT_MODE, COOKIE_PRIORITY_DEFAULT));
@@ skipping 10 matching lines @@
   // The store should also delete the session cookie. Wait until that has been
   // done.
   DestroyStore();
 
   // Create a store that loads old session cookies and test that the session
   // cookie is gone.
   CreateAndLoad(false, true, &cookies);
   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);
   static const char kSessionName[] = "session";
   static const char kPersistentName[] = "persistent";
 
   // Add a session cookie.
   store_->AddCookie(CanonicalCookie(
       kSessionName, "val", "sessioncookie.com", "/", base::Time::Now(),
       base::Time(), base::Time(), false, false, CookieSameSite::DEFAULT_MODE,
@@ skipping 299 matching lines @@
   EXPECT_TRUE(was_called_with_no_cookies);
 
   // Same with trying to load a specific cookie.
   was_called_with_no_cookies = false;
   store_->LoadCookiesForKey("foo.bar", base::Bind(WasCalledWithNoCookies,
                                                   &was_called_with_no_cookies));
   EXPECT_TRUE(was_called_with_no_cookies);
 }
 
 }  // namespace net