Password manager internals page: Improve security

components/autofill/core/common/save_password_progress_logger_unittest.cc

Index: components/autofill/core/common/save_password_progress_logger_unittest.cc
diff --git a/components/autofill/core/common/save_password_progress_logger_unittest.cc b/components/autofill/core/common/save_password_progress_logger_unittest.cc
index 9c0950562a1d1fd287efafbb0a221bfda3b6c09c..eb6dedf18883264ee2732c4d67bdfa9eee255a05 100644
--- a/components/autofill/core/common/save_password_progress_logger_unittest.cc
+++ b/components/autofill/core/common/save_password_progress_logger_unittest.cc
@@ -21,22 +21,28 @@ namespace autofill {
 
 namespace {
 
-const char kTestString[] = "Test";
+const char kTestString[] = "Message";  // Corresponds to STRING_MESSAGE.
 
 class TestLogger : public SavePasswordProgressLogger {
  public:
   bool LogsContainSubstring(const std::string& substring) {
-    return accumulated_log_.find(substring) != std::string::npos;
+    return GetAccumulatedLog().find(substring) != std::string::npos;
   }
 
-  std::string accumulated_log() { return accumulated_log_; }
+  std::string GetAccumulatedLog() {
+    return SanitizeStructuredLogs(accumulated_logs_);
+  }
+
+  std::vector<SavePasswordProgressLogger::StructuredLog> accumulated_logs() {
+    return accumulated_logs_;
+  }
 
  private:
-  virtual void SendLog(const std::string& log) OVERRIDE {
-    accumulated_log_.append(log);
+  virtual void SendLog(const std::vector<StructuredLog>& logs) OVERRIDE {
+    accumulated_logs_ = logs;
   }
 
-  std::string accumulated_log_;
+  std::vector<SavePasswordProgressLogger::StructuredLog> accumulated_logs_;
 };
 
 };  // namespace
@@ -45,36 +51,39 @@ TEST(SavePasswordProgressLoggerTest, LogPasswordForm) {
   TestLogger logger;
   PasswordForm form;
   form.action = GURL("http://example.org/verysecret?verysecret");
+  form.password_element = UTF8ToUTF16("pwdelement");
   form.password_value = UTF8ToUTF16("verysecret");
   form.username_value = UTF8ToUTF16("verysecret");
-  logger.LogPasswordForm(kTestString, form);
+  logger.LogPasswordForm(SavePasswordProgressLogger::STRING_MESSAGE, form);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
+  EXPECT_TRUE(logger.LogsContainSubstring("pwdelement"));
   EXPECT_TRUE(logger.LogsContainSubstring("http://example.org"));
   EXPECT_FALSE(logger.LogsContainSubstring("verysecret"));
 }
 
 TEST(SavePasswordProgressLoggerTest, LogHTMLForm) {
   TestLogger logger;
-  logger.LogHTMLForm(kTestString,
-                     "form_name",
-                     "form_method",
+  logger.LogHTMLForm(SavePasswordProgressLogger::STRING_MESSAGE,
+                     "formname",
+                     "post",
                      GURL("http://example.org/verysecret?verysecret"));
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
-  EXPECT_TRUE(logger.LogsContainSubstring("form_name"));
-  EXPECT_TRUE(logger.LogsContainSubstring("form_method"));
+  EXPECT_TRUE(logger.LogsContainSubstring("formname"));
+  EXPECT_TRUE(logger.LogsContainSubstring("POST"));
   EXPECT_TRUE(logger.LogsContainSubstring("http://example.org"));
   EXPECT_FALSE(logger.LogsContainSubstring("verysecret"));
 }
 
 TEST(SavePasswordProgressLoggerTest, LogURL) {
   TestLogger logger;
-  logger.LogURL(kTestString, GURL("http://example.org/verysecret?verysecret"));
+  logger.LogURL(SavePasswordProgressLogger::STRING_MESSAGE,
+                GURL("http://example.org/verysecret?verysecret"));
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
   EXPECT_TRUE(logger.LogsContainSubstring("http://example.org"));
   EXPECT_FALSE(logger.LogsContainSubstring("verysecret"));
@@ -82,18 +91,18 @@ TEST(SavePasswordProgressLoggerTest, LogURL) {
 
 TEST(SavePasswordProgressLoggerTest, LogBooleanTrue) {
   TestLogger logger;
-  logger.LogBoolean(kTestString, true);
+  logger.LogBoolean(SavePasswordProgressLogger::STRING_MESSAGE, true);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
   EXPECT_TRUE(logger.LogsContainSubstring("true"));
 }
 
 TEST(SavePasswordProgressLoggerTest, LogBooleanFalse) {
   TestLogger logger;
-  logger.LogBoolean(kTestString, false);
+  logger.LogBoolean(SavePasswordProgressLogger::STRING_MESSAGE, false);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
   EXPECT_TRUE(logger.LogsContainSubstring("false"));
 }
@@ -101,9 +110,9 @@ TEST(SavePasswordProgressLoggerTest, LogBooleanFalse) {
 TEST(SavePasswordProgressLoggerTest, LogSignedNumber) {
   TestLogger logger;
   int signed_number = -12345;
-  logger.LogNumber(kTestString, signed_number);
+  logger.LogNumber(SavePasswordProgressLogger::STRING_MESSAGE, signed_number);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
   EXPECT_TRUE(logger.LogsContainSubstring("-12345"));
 }
@@ -111,43 +120,45 @@ TEST(SavePasswordProgressLoggerTest, LogSignedNumber) {
 TEST(SavePasswordProgressLoggerTest, LogUnsignedNumber) {
   TestLogger logger;
   size_t unsigned_number = 654321;
-  logger.LogNumber(kTestString, unsigned_number);
+  logger.LogNumber(SavePasswordProgressLogger::STRING_MESSAGE, unsigned_number);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
   EXPECT_TRUE(logger.LogsContainSubstring("654321"));
 }
 
-TEST(SavePasswordProgressLoggerTest, LogFinalDecisionSave) {
-  TestLogger logger;
-  logger.LogFinalDecision(SavePasswordProgressLogger::DECISION_SAVE);
-  SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
-  EXPECT_TRUE(logger.LogsContainSubstring("SAVE"));
-}
-
-TEST(SavePasswordProgressLoggerTest, LogFinalDecisionAsk) {
-  TestLogger logger;
-  logger.LogFinalDecision(SavePasswordProgressLogger::DECISION_ASK);
-  SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
-  EXPECT_TRUE(logger.LogsContainSubstring("ASK"));
-}
-
-TEST(SavePasswordProgressLoggerTest, LogFinalDecisionDrop) {
+TEST(SavePasswordProgressLoggerTest, LogMessage) {
   TestLogger logger;
-  logger.LogFinalDecision(SavePasswordProgressLogger::DECISION_DROP);
+  logger.LogMessage(SavePasswordProgressLogger::STRING_MESSAGE);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
-  EXPECT_TRUE(logger.LogsContainSubstring("DROP"));
+                                  << logger.GetAccumulatedLog() << "]");
+  EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
 }
 
-TEST(SavePasswordProgressLoggerTest, LogMessage) {
+TEST(SavePasswordProgressLoggerTest, SanitizeStructuredLogs) {
+  // The sanitizing method actually letting safe content through is tested in
+  // the rest of this file. The same goes for scrubbing URLs. Here we test that
+  // element IDs, the only source of free strings, is properly filtered.
   TestLogger logger;
-  logger.LogMessage(kTestString);
+  PasswordForm form;
+  const std::string kHTMLInside("Username <script> element");
+  const std::string kHTMLInsideExpected("username  script  element");
+  const std::string kIPAddressInside("y128.0.0.1Y");
+  const std::string kIPAddressInsideExpected("y128 0 0 1y");
+  const std::string kSpecialCharsInside("X@#a$%B&*c()D;:e+-x");
+  const std::string kSpecialCharsInsideExpected("x  a  b  c  d  e  x");
+  form.username_element = UTF8ToUTF16(kHTMLInside);
+  form.password_element = UTF8ToUTF16(kIPAddressInside);
+  form.old_password_element = UTF8ToUTF16(kSpecialCharsInside);
+  logger.LogPasswordForm(SavePasswordProgressLogger::STRING_MESSAGE, form);
   SCOPED_TRACE(testing::Message() << "Log string = ["
-                                  << logger.accumulated_log() << "]");
+                                  << logger.GetAccumulatedLog() << "]");
   EXPECT_TRUE(logger.LogsContainSubstring(kTestString));
+  EXPECT_FALSE(logger.LogsContainSubstring(kHTMLInside));
+  EXPECT_TRUE(logger.LogsContainSubstring(kHTMLInsideExpected));
+  EXPECT_FALSE(logger.LogsContainSubstring(kIPAddressInside));
+  EXPECT_TRUE(logger.LogsContainSubstring(kIPAddressInsideExpected));
+  EXPECT_FALSE(logger.LogsContainSubstring(kSpecialCharsInside));
+  EXPECT_TRUE(logger.LogsContainSubstring(kSpecialCharsInsideExpected));
 }
-
 }  // namespace autofill