[Top Sites] Encoding redirects field in TopSitesDatabase, and adding validations

chrome/browser/history/top_sites_database_unittest.cc

Index: chrome/browser/history/top_sites_database_unittest.cc
diff --git a/chrome/browser/history/top_sites_database_unittest.cc b/chrome/browser/history/top_sites_database_unittest.cc
index 9ec671964e2d4866b7fc9b387885ea3d60ce911b..608c6dd8d258a861af025eb1bc865d650f18c599 100644
--- a/chrome/browser/history/top_sites_database_unittest.cc
+++ b/chrome/browser/history/top_sites_database_unittest.cc
@@ -2,14 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <map>
+#include "chrome/browser/history/top_sites_database.h"
 
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/history/history_types.h"
-#include "chrome/browser/history/top_sites_database.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/tools/profiles/thumbnail-inl.h"
 #include "sql/connection.h"
@@ -81,6 +80,11 @@ class TopSitesDatabaseTest : public testing::Test {
     file_name_ = temp_dir_.path().AppendASCII("TestTopSites.db");
   }
 
+  // The only difference between v3 and v4 is how redirects URLs are stored, so
+  // most of the tests should be common.
+  void DatabaseTest3And4Common();
+  void RecoveryTest3And4Common();
+
   base::ScopedTempDir temp_dir_;
   base::FilePath file_name_;
 };
@@ -123,33 +127,57 @@ TEST_F(TopSitesDatabaseTest, Version2) {
   ASSERT_EQ(2u, thumbnails.size());
 }
 
-TEST_F(TopSitesDatabaseTest, Version3) {
-  ASSERT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v3.sql"));
-
+void TopSitesDatabaseTest::DatabaseTest3And4Common() {
   TopSitesDatabase db;
   ASSERT_TRUE(db.Init(file_name_));
-
   VerifyTablesAndColumns(db.db_.get());
 
-  // Basic operational check.
+  // Basic read operational check.
   MostVisitedURLList urls;
   std::map<GURL, Images> thumbnails;
   db.GetPageThumbnails(&urls, &thumbnails);
   ASSERT_EQ(3u, urls.size());
   ASSERT_EQ(3u, thumbnails.size());
   EXPECT_EQ(kUrl0, urls[0].url);  // [0] because of url_rank.
+  EXPECT_EQ(kUrl1, urls[1].url);
+  EXPECT_EQ(kUrl2, urls[2].url);
+  EXPECT_EQ(2u, urls[0].redirects.size());
+  EXPECT_EQ(1u, urls[1].redirects.size());
+  EXPECT_EQ(1u, urls[2].redirects.size());
+
   // kGoogleThumbnail includes nul terminator.
   ASSERT_EQ(sizeof(kGoogleThumbnail) - 1,
             thumbnails[urls[0].url].thumbnail->size());
   EXPECT_TRUE(!memcmp(thumbnails[urls[0].url].thumbnail->front(),
                       kGoogleThumbnail, sizeof(kGoogleThumbnail) - 1));
 
+  // Basic write operational check.
   ASSERT_TRUE(db.RemoveURL(urls[1]));
   db.GetPageThumbnails(&urls, &thumbnails);
   ASSERT_EQ(2u, urls.size());
   ASSERT_EQ(2u, thumbnails.size());
 }
 
+TEST_F(TopSitesDatabaseTest, Version3) {
+  ASSERT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v3.sql"));
+  DatabaseTest3And4Common();
+}
+
+TEST_F(TopSitesDatabaseTest, Version4) {
+  ASSERT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v4.sql"));
+  DatabaseTest3And4Common();
+}
+
+// Verify that database is unusable at the SQLite level.
+void RecoveryTestVerifyDatabaseIsUnusable(const base::FilePath& file_name) {
+  sql::ScopedErrorIgnorer ignore_errors;
+  ignore_errors.IgnoreError(SQLITE_CORRUPT);
+  sql::Connection raw_db;
+  EXPECT_TRUE(raw_db.Open(file_name));
+  EXPECT_FALSE(raw_db.IsSQLValid("PRAGMA integrity_check"));
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
+}
+
 // Version 1 is deprecated, the resulting schema should be current,
 // with no data.
 TEST_F(TopSitesDatabaseTest, Recovery1) {
@@ -163,15 +191,7 @@ TEST_F(TopSitesDatabaseTest, Recovery1) {
   // Corrupt the database by adjusting the header size.
   EXPECT_TRUE(sql::test::CorruptSizeInHeader(file_name_));
 
-  // Database is unusable at the SQLite level.
-  {
-    sql::ScopedErrorIgnorer ignore_errors;
-    ignore_errors.IgnoreError(SQLITE_CORRUPT);
-    sql::Connection raw_db;
-    EXPECT_TRUE(raw_db.Open(file_name_));
-    EXPECT_FALSE(raw_db.IsSQLValid("PRAGMA integrity_check"));
-    ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
-  }
+  RecoveryTestVerifyDatabaseIsUnusable(file_name_);
 
   // Corruption should be detected and recovered during Init().
   {
@@ -198,15 +218,7 @@ TEST_F(TopSitesDatabaseTest, Recovery2) {
   // Corrupt the database by adjusting the header.
   EXPECT_TRUE(sql::test::CorruptSizeInHeader(file_name_));
 
-  // Database is unusable at the SQLite level.
-  {
-    sql::ScopedErrorIgnorer ignore_errors;
-    ignore_errors.IgnoreError(SQLITE_CORRUPT);
-    sql::Connection raw_db;
-    EXPECT_TRUE(raw_db.Open(file_name_));
-    EXPECT_FALSE(raw_db.IsSQLValid("PRAGMA integrity_check"));
-    ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
-  }
+  RecoveryTestVerifyDatabaseIsUnusable(file_name_);
 
   // Corruption should be detected and recovered during Init().  After recovery,
   // the Version2 checks should work.
@@ -216,7 +228,6 @@ TEST_F(TopSitesDatabaseTest, Recovery2) {
 
     TopSitesDatabase db;
     ASSERT_TRUE(db.Init(file_name_));
-
     VerifyTablesAndColumns(db.db_.get());
 
     // Basic operational check.
@@ -236,26 +247,11 @@ TEST_F(TopSitesDatabaseTest, Recovery2) {
   }
 }
 
-TEST_F(TopSitesDatabaseTest, Recovery3) {
-  // Recovery module only supports some platforms at this time.
-  if (!sql::Recovery::FullRecoverySupported())
-    return;
-
-  // Create an example database.
-  EXPECT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v3.sql"));
-
+void TopSitesDatabaseTest::RecoveryTest3And4Common() {
   // Corrupt the database by adjusting the header.
   EXPECT_TRUE(sql::test::CorruptSizeInHeader(file_name_));
 
-  // Database is unusable at the SQLite level.
-  {
-    sql::ScopedErrorIgnorer ignore_errors;
-    ignore_errors.IgnoreError(SQLITE_CORRUPT);
-    sql::Connection raw_db;
-    EXPECT_TRUE(raw_db.Open(file_name_));
-    EXPECT_FALSE(raw_db.IsSQLValid("PRAGMA integrity_check"));
-    ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
-  }
+  RecoveryTestVerifyDatabaseIsUnusable(file_name_);
 
   // Corruption should be detected and recovered during Init().
   {
@@ -265,6 +261,7 @@ TEST_F(TopSitesDatabaseTest, Recovery3) {
     TopSitesDatabase db;
     ASSERT_TRUE(db.Init(file_name_));
 
+    // Basic operational check.
     MostVisitedURLList urls;
     std::map<GURL, Images> thumbnails;
     db.GetPageThumbnails(&urls, &thumbnails);
@@ -352,6 +349,28 @@ TEST_F(TopSitesDatabaseTest, Recovery3) {
   }
 }
 
+TEST_F(TopSitesDatabaseTest, Recovery3) {
+  // Recovery module only supports some platforms at this time.
+  if (!sql::Recovery::FullRecoverySupported())
+    return;
+
+  // Create an example database.
+  EXPECT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v3.sql"));
+
+  RecoveryTest3And4Common();
+}
+
+TEST_F(TopSitesDatabaseTest, Recovery4) {
+  // Recovery module only supports some platforms at this time.
+  if (!sql::Recovery::FullRecoverySupported())
+    return;
+
+  // Create an example database.
+  EXPECT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v4.sql"));
+
+  RecoveryTest3And4Common();
+}
+
 TEST_F(TopSitesDatabaseTest, AddRemoveEditThumbnails) {
   ASSERT_TRUE(CreateDatabaseFromSQL(file_name_, "TopSites.v3.sql"));
 
@@ -476,4 +495,205 @@ TEST_F(TopSitesDatabaseTest, AddRemoveEditThumbnails) {
   EXPECT_EQ(kUrl0, urls[1].url);
 }
 
+TEST_F(TopSitesDatabaseTest, EncodeCSVString) {
+  std::vector<std::string> empty_list;
+  EXPECT_EQ("", TopSitesDatabase::EncodeCSVString(empty_list));
+
+  std::vector<std::string> empty_string_list;
+  empty_string_list.push_back("");
+  EXPECT_EQ("\"\"", TopSitesDatabase::EncodeCSVString(empty_string_list));
+  empty_string_list.push_back("");
+  EXPECT_EQ("\"\",\"\"", TopSitesDatabase::EncodeCSVString(empty_string_list));
+
+  // "\n" is treated like any other character.
+  std::vector<std::string> single_list;
+  single_list.push_back("abczABCZ-123 !@#$%^&*(),.'\"\n\r_");
+  EXPECT_EQ("\"abczABCZ-123 !@#$%^&*(),.'\"\"\n\r_\"",
+            TopSitesDatabase::EncodeCSVString(single_list));
+
+  std::vector<std::string> double_list;
+  double_list.push_back(" a \"string\", this is ");
+  double_list.push_back("http://www.test.com:80/path?query#ref");
+  EXPECT_EQ("\" a \"\"string\"\", this is \","
+            "\"http://www.test.com:80/path?query#ref\"",
+            TopSitesDatabase::EncodeCSVString(double_list));
+
+  std::vector<std::string> multi_list;
+  multi_list.push_back("test");
+  multi_list.push_back("");
+  multi_list.push_back(",");
+  multi_list.push_back("http://www.google.com");
+  multi_list.push_back(" ");
+  multi_list.push_back("\r\n");
+  multi_list.push_back("\"");
+  multi_list.push_back("\"quote\"");
+  multi_list.push_back("data:text/plain,this text  has space");
+  EXPECT_EQ("\"test\",\"\",\",\",\"http://www.google.com\","
+            "\" \",\"\r\n\",\"\"\"\",\"\"\"quote\"\"\","
+            "\"data:text/plain,this text  has space\"",
+            TopSitesDatabase::EncodeCSVString(multi_list));
+}
+
+TEST_F(TopSitesDatabaseTest, DecodeCSVStringSucceed) {
+  std::vector<std::string> res;
+  res.push_back("this gets overwritten");
+
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString("", &res));
+  EXPECT_TRUE(res.empty());
+
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString("\"\"", &res));
+  EXPECT_EQ(1U, res.size());
+  EXPECT_EQ("", res[0]);
+
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString("\"\",\"\"", &res));
+  EXPECT_EQ(2U, res.size());
+  EXPECT_EQ("", res[0]);
+  EXPECT_EQ("", res[1]);
+
+  // "\n" is treated like any other character.
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString(
+                  "\"abczABCZ-123 !@#$%^&*(),.'\"\"\n\r_\"", &res));
+  EXPECT_EQ(1U, res.size());
+  EXPECT_EQ("abczABCZ-123 !@#$%^&*(),.'\"\n\r_", res[0]);
+
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString(
+                  "\" a \"\"string\"\", this is \","
+                  "\"http://www.test.com:80/path?query#ref\"", &res));
+  EXPECT_EQ(2U, res.size());
+  EXPECT_EQ(" a \"string\", this is ", res[0]);
+  EXPECT_EQ("http://www.test.com:80/path?query#ref", res[1]);
+
+  EXPECT_TRUE(TopSitesDatabase::DecodeCSVString(
+                  "\"test\",\"\",\",\",\"http://www.google.com\","
+                  "\" \",\"\r\n\",\"\"\"\",\"\"\"quote\"\"\","
+                  "\"data:text/plain,this text  has space\"", &res));
+  EXPECT_EQ(9U, res.size());
+  EXPECT_EQ("test", res[0]);
+  EXPECT_EQ("", res[1]);
+  EXPECT_EQ(",", res[2]);
+  EXPECT_EQ("http://www.google.com", res[3]);
+  EXPECT_EQ(" ", res[4]);
+  EXPECT_EQ("\r\n", res[5]);
+  EXPECT_EQ("\"", res[6]);
+  EXPECT_EQ("\"quote\"", res[7]);
+  EXPECT_EQ("data:text/plain,this text  has space", res[8]);
+}
+
+TEST_F(TopSitesDatabaseTest, DecodeCSVStringFail) {
+  std::vector<std::string> dummy;
+  dummy.push_back("Can't touch this");
+  // Missing quotes.
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("test", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("test,blah", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"test\",blah", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"test", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("test\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"\"\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"\",\"\"\"", &dummy));
+  // Misplaced comma.
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString(",\"A\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"A\",", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"A\",,\"B\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"A\",\"B\",\"C\",", &dummy));
+  // Garbage after quote.
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"test\"a\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"\"test\"", &dummy));
+  // We're strict with trailing white spaces.
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"no\", \"space\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"no\" ,\"space\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString(" \"bad\"", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"bad\" ", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString("\"bad\"\n", &dummy));
+  // Other.
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString(",", &dummy));
+  EXPECT_FALSE(TopSitesDatabase::DecodeCSVString(",,,,,,", &dummy));
+  // Verify |dummy| is not touched.
+  EXPECT_EQ(1U, dummy.size());
+  EXPECT_EQ("Can't touch this", dummy[0]);
+}
+
+TEST_F(TopSitesDatabaseTest, EncodeRedirects) {
+  RedirectList redirects1;
+  EXPECT_EQ("", TopSitesDatabase::EncodeRedirects(redirects1));
+  redirects1.push_back(GURL("http://www.google.com"));
+  EXPECT_EQ("\"http://www.google.com/\"",
+            TopSitesDatabase::EncodeRedirects(redirects1));
+  redirects1.push_back(GURL("https://www.youtube.ca/"));
+  EXPECT_EQ("\"http://www.google.com/\",\"https://www.youtube.ca/\"",
+            TopSitesDatabase::EncodeRedirects(redirects1));
+  redirects1.push_back(GURL("http://www.test.org:137/path?query#ref"));
+  EXPECT_EQ("\"http://www.google.com/\",\"https://www.youtube.ca/\","
+            "\"http://www.test.org:137/path?query#ref\"",
+            TopSitesDatabase::EncodeRedirects(redirects1));
+
+  RedirectList redirects2;
+  redirects2.push_back(GURL("http://www.google.com"));
+  // Adding invalid URL.
+  redirects2.push_back(GURL("this is not valid URL"));
+  // Adding data URL.
+  redirects2.push_back(GURL("data:text/plain,\"this has space\""));
+  // Invalid URL should be ignored.
+  EXPECT_EQ("\"http://www.google.com/\","
+            "\"data:text/plain,\"\"this has space\"\"\"",
+            TopSitesDatabase::EncodeRedirects(redirects2));
+}
+
+TEST_F(TopSitesDatabaseTest, DecodeRedirects) {
+  RedirectList redirects1;
+  TopSitesDatabase::DecodeRedirects("", &redirects1);
+  EXPECT_EQ(0u, redirects1.size());
+
+  RedirectList redirects2;
+  TopSitesDatabase::DecodeRedirects(
+      "\"http://www.google.com/\",\"https://www.youtube.ca/\","
+      "\"http://www.test.org:137/path?query#ref\"", &redirects2);
+  EXPECT_EQ(3U, redirects2.size());
+  EXPECT_EQ(GURL("http://www.google.com"), redirects2[0]);
+  EXPECT_EQ(GURL("https://www.youtube.ca/"), redirects2[1]);
+  EXPECT_EQ(GURL("http://www.test.org:137/path?query#ref"), redirects2[2]);
+
+  RedirectList redirects3;
+  TopSitesDatabase::DecodeRedirects(
+      "\"http://www.google.com\","
+      "\"this is not valid URL\",\"\","
+      "\"data:text/plain,\"\"this has space\"\"\"", &redirects3);
+  EXPECT_EQ(2U, redirects3.size());
+  EXPECT_EQ(GURL("http://www.google.com"), redirects3[0]);
+  EXPECT_EQ(GURL("data:text/plain,\"this has space\""), redirects3[1]);
+}
+
+// Fallback case when redirects are not stored as CSV strings.
+TEST_F(TopSitesDatabaseTest, DecodeRedirectsFallback) {
+  RedirectList redirects1;
+  TopSitesDatabase::DecodeRedirects("http://www.chromium.org", &redirects1);
+  EXPECT_EQ(1u, redirects1.size());
+  EXPECT_EQ(GURL("http://www.chromium.org"), redirects1[0]);
+
+  // Valid case, note multiple spaces.
+  RedirectList redirects2;
+  TopSitesDatabase::DecodeRedirects(
+        " http://www.google.com    https://www.youtube.ca"
+        " http://www.test.org:80/path?query#ref  ",
+        &redirects2);
+  EXPECT_EQ(3u, redirects2.size());
+  EXPECT_EQ(GURL("http://www.google.com"), redirects2[0]);
+  EXPECT_EQ(GURL("https://www.youtube.ca"), redirects2[1]);
+  EXPECT_EQ(GURL("http://www.test.org:80/path?query#ref"), redirects2[2]);
+
+  // This might appear in old database, and should be ignored.
+  RedirectList redirects3;
+  TopSitesDatabase::DecodeRedirects("data:text/plain,this text  has space",
+                                    &redirects3);
+  EXPECT_EQ(1u, redirects3.size());
+  EXPECT_EQ(GURL("data:text/plain,this"), redirects3[0]);
+
+  // Unfortunate case that creates dubious results.
+  RedirectList redirects4;
+  TopSitesDatabase::DecodeRedirects("Data:text/plain,this http://a has space",
+                                    &redirects4);
+  EXPECT_EQ(2u, redirects4.size());
+  EXPECT_EQ(GURL("Data:text/plain,this"), redirects4[0]);
+  EXPECT_EQ(GURL("http://a"), redirects4[1]);
+}
+
 }  // namespace history