Index: components/autofill/core/common/save_password_progress_logger.cc |
diff --git a/components/autofill/core/common/save_password_progress_logger.cc b/components/autofill/core/common/save_password_progress_logger.cc |
index cdb8dd7a177e97f1542db30a714749683b3094f1..a580b827b463c013c2f1f366c56fef0e805429a0 100644 |
--- a/components/autofill/core/common/save_password_progress_logger.cc |
+++ b/components/autofill/core/common/save_password_progress_logger.cc |
@@ -4,9 +4,15 @@ |
#include "components/autofill/core/common/save_password_progress_logger.h" |
+#include <algorithm> |
+ |
#include "base/json/json_writer.h" |
#include "base/logging.h" |
+#include "base/memory/scoped_ptr.h" |
#include "base/numerics/safe_conversions.h" |
+#include "base/strings/string16.h" |
+#include "base/strings/string_util.h" |
+#include "base/strings/utf_string_conversions.h" |
#include "base/values.h" |
#include "components/autofill/core/common/password_form.h" |
@@ -20,6 +26,76 @@ namespace autofill { |
namespace { |
+// Note 1: Caching the ID->string map in an array would be probably faster, but |
+// the switch statement is (a) robust against re-ordering, and (b) checks in |
+// compile-time, that all IDs get a string assigned. The expected frequency of |
+// calls is low enough (in particular, zero if password manager internals page |
+// is not open), that optimizing for code robustness is preferred agains speed. |
Ilya Sherman
2014/04/16 22:02:11
nit: "agains" -> "against"
vabr (Chromium)
2014/04/17 12:33:09
Done.
|
+// Note 2: The messages can be used as dictionary keys. Do not use '.' in them. |
+std::string GetStringFromID(SavePasswordProgressLogger::StringID id) { |
+ switch (id) { |
+ case SavePasswordProgressLogger::STRING_DECISION_ASK: |
+ return "Decision: ASK the user"; |
+ case SavePasswordProgressLogger::STRING_DECISION_DROP: |
+ return "Decision: DROP the password"; |
+ case SavePasswordProgressLogger::STRING_DECISION_SAVE: |
+ return "Decision: SAVE the password"; |
+ case SavePasswordProgressLogger::STRING_METHOD: |
+ return "Form method"; |
+ case SavePasswordProgressLogger::STRING_METHOD_GET: |
+ return "GET"; |
+ case SavePasswordProgressLogger::STRING_METHOD_POST: |
+ return "POST"; |
+ case SavePasswordProgressLogger::STRING_METHOD_EMPTY: |
+ return "(empty)"; |
+ case SavePasswordProgressLogger::STRING_OTHER: |
+ return "(other)"; |
+ case SavePasswordProgressLogger::STRING_SCHEME_HTML: |
+ return "HTML"; |
+ case SavePasswordProgressLogger::STRING_SCHEME_BASIC: |
+ return "Basic"; |
+ case SavePasswordProgressLogger::STRING_SCHEME_DIGEST: |
+ return "Digest"; |
+ case SavePasswordProgressLogger::STRING_SCHEME_MESSAGE: |
+ return "Scheme"; |
+ case SavePasswordProgressLogger::STRING_SIGNON_REALM: |
+ return "Signon realm"; |
+ case SavePasswordProgressLogger::STRING_ORIGINAL_SIGNON_REALM: |
+ return "Original signon realm"; |
+ case SavePasswordProgressLogger::STRING_ORIGIN: |
+ return "Origin"; |
+ case SavePasswordProgressLogger::STRING_ACTION: |
+ return "Action"; |
+ case SavePasswordProgressLogger::STRING_USERNAME_ELEMENT: |
+ return "Username element"; |
+ case SavePasswordProgressLogger::STRING_PASSWORD_ELEMENT: |
+ return "Password element"; |
+ case SavePasswordProgressLogger::STRING_PASSWORD_AUTOCOMPLETE_SET: |
+ return "Password autocomplete set"; |
+ case SavePasswordProgressLogger::STRING_OLD_PASSWORD_ELEMENT: |
+ return "Old password element"; |
+ case SavePasswordProgressLogger::STRING_SSL_VALID: |
+ return "SSL valid"; |
+ case SavePasswordProgressLogger::STRING_PASSWORD_GENERATED: |
+ return "Password generated"; |
+ case SavePasswordProgressLogger::STRING_TIMES_USED: |
+ return "Times used"; |
+ case SavePasswordProgressLogger::STRING_USE_ADDITIONAL_AUTHENTICATION: |
+ return "Use additional authentication"; |
+ case SavePasswordProgressLogger::STRING_PSL_MATCH: |
+ return "PSL match"; |
+ case SavePasswordProgressLogger::STRING_NAME_OR_ID: |
+ return "Form name or ID"; |
+ case SavePasswordProgressLogger::STRING_MESSAGE: |
+ return "Message"; |
+ case SavePasswordProgressLogger::STRING_INVALID: |
+ return "INVALID"; |
+ // Intentionally no default: clause here -- all IDs need to get covered. |
+ } |
+ NOTREACHED(); // Win compilers don't believe this is unreachable. |
+ return NULL; |
Ilya Sherman
2014/04/16 22:02:11
NULL isn't a valid std::string. I think you meant
vabr (Chromium)
2014/04/17 12:33:09
Thanks! That was a relic from the time this return
|
+}; |
+ |
// Removes privacy sensitive parts of |url| (currently all but host and scheme). |
std::string ScrubURL(const GURL& url) { |
if (url.is_valid()) |
@@ -27,112 +103,173 @@ std::string ScrubURL(const GURL& url) { |
return std::string(); |
} |
-std::string FormSchemeToString(PasswordForm::Scheme scheme) { |
+// Returns true for all characters which we don't want to see in the logged IDs |
+// or names of HTML elements. |
+bool IsUnwantedInElementID(char c) { |
+ return !IsAsciiAlpha(c) && !IsAsciiDigit(c); |
+} |
+ |
+// Replaces all characters satisfying IsUnwantedInElementID by a ' ', and turns |
+// all characters to lowercase. This damages some valid HTML element IDs or |
+// names, but it is likely that it will be still possible to match the scrubbed |
+// string to the original ID or name in the HTML doc. That's good enough for the |
+// logging purposes, and provides some security benefits. |
+std::string ScrubElementID(std::string element_id) { |
+ std::replace_if( |
+ element_id.begin(), element_id.end(), IsUnwantedInElementID, ' '); |
+ return StringToLowerASCII(element_id); |
+} |
+ |
+SavePasswordProgressLogger::StringID FormSchemeToStringID( |
+ PasswordForm::Scheme scheme) { |
switch (scheme) { |
case PasswordForm::SCHEME_HTML: |
- return "HTML"; |
+ return SavePasswordProgressLogger::STRING_SCHEME_HTML; |
case PasswordForm::SCHEME_BASIC: |
- return "BASIC"; |
+ return SavePasswordProgressLogger::STRING_SCHEME_BASIC; |
case PasswordForm::SCHEME_DIGEST: |
- return "DIGEST"; |
+ return SavePasswordProgressLogger::STRING_SCHEME_DIGEST; |
case PasswordForm::SCHEME_OTHER: |
- return "OTHER"; |
+ return SavePasswordProgressLogger::STRING_OTHER; |
} |
NOTREACHED(); // Win compilers don't believe this is unreachable. |
- return std::string(); |
+ return SavePasswordProgressLogger::StringID::STRING_INVALID; |
} |
-StringValue DecisionToStringValue( |
- SavePasswordProgressLogger::Decision decision) { |
- switch (decision) { |
- case SavePasswordProgressLogger::DECISION_SAVE: |
- return StringValue("SAVE the password"); |
- case SavePasswordProgressLogger::DECISION_ASK: |
- return StringValue("ASK the user whether to save the password"); |
- case SavePasswordProgressLogger::DECISION_DROP: |
- return StringValue("DROP the password"); |
- } |
- NOTREACHED(); // Win compilers don't believe this is unreachable. |
- return StringValue(std::string()); |
+SavePasswordProgressLogger::StringID FormMethodToStringID( |
+ const std::string& method) { |
+ std::string method_processed; |
+ base::TrimWhitespaceASCII( |
+ StringToLowerASCII(method), base::TRIM_ALL, &method_processed); |
+ if (method_processed.empty()) |
+ return SavePasswordProgressLogger::STRING_METHOD_EMPTY; |
+ else if (method_processed == "get") |
+ return SavePasswordProgressLogger::STRING_METHOD_GET; |
+ else if (method_processed == "post") |
+ return SavePasswordProgressLogger::STRING_METHOD_POST; |
+ else |
+ return SavePasswordProgressLogger::STRING_OTHER; |
+} |
+ |
+// GetValueFromLog: Sanitize the arguments, and return them as base::Value. |
+scoped_ptr<Value> GetValueFromLog(const GURL& url) { |
+ return scoped_ptr<Value>(Value::CreateStringValue(ScrubURL(url))); |
+} |
+ |
+scoped_ptr<Value> GetValueFromLog( |
+ SavePasswordProgressLogger::StringID message) { |
+ return scoped_ptr<Value>(Value::CreateStringValue(GetStringFromID(message))); |
+} |
+ |
+scoped_ptr<Value> GetValueFromLog(int number) { |
+ return scoped_ptr<Value>(Value::CreateIntegerValue(number)); |
+} |
+ |
+scoped_ptr<Value> GetValueFromLog(bool truth_value) { |
+ return scoped_ptr<Value>(Value::CreateBooleanValue(truth_value)); |
+} |
+ |
+scoped_ptr<Value> GetValueFromLog(const std::string& element_id) { |
+ return scoped_ptr<Value>( |
+ Value::CreateStringValue(ScrubElementID(element_id))); |
+} |
+ |
+scoped_ptr<Value> GetValueFromLog(const base::string16& element_id) { |
+ return scoped_ptr<Value>( |
+ Value::CreateStringValue(ScrubElementID(UTF16ToUTF8(element_id)))); |
+} |
+ |
+template <class T> |
+void AddToDict(DictionaryValue* dict, |
+ SavePasswordProgressLogger::StringID label, |
+ T log) { |
+ dict->Set(GetStringFromID(label), GetValueFromLog(log).release()); |
} |
Ilya Sherman
2014/04/16 22:02:11
Hmm, I'm not thrilled about the addition of this m
vabr (Chromium)
2014/04/17 12:33:09
As for the style guide, it says: "Use overloaded f
|
} // namespace |
-SavePasswordProgressLogger::SavePasswordProgressLogger() {} |
+SavePasswordProgressLogger::SavePasswordProgressLogger() { |
+} |
-SavePasswordProgressLogger::~SavePasswordProgressLogger() {} |
+SavePasswordProgressLogger::~SavePasswordProgressLogger() { |
+} |
-void SavePasswordProgressLogger::LogPasswordForm(const std::string& message, |
- const PasswordForm& form) { |
- DictionaryValue log; |
- // Do not use the "<<" operator for PasswordForms, because it also prints |
- // passwords. Also, that operator is only for testing. |
- log.SetString("scheme", FormSchemeToString(form.scheme)); |
- log.SetString("signon realm", ScrubURL(GURL(form.signon_realm))); |
- log.SetString("original signon realm", |
- ScrubURL(GURL(form.original_signon_realm))); |
- log.SetString("origin", ScrubURL(form.origin)); |
- log.SetString("action", ScrubURL(form.action)); |
- log.SetString("username element", form.username_element); |
- log.SetString("password element", form.password_element); |
- log.SetBoolean("password autocomplete set", form.password_autocomplete_set); |
- log.SetString("old password element", form.old_password_element); |
- log.SetBoolean("ssl valid", form.ssl_valid); |
- log.SetBoolean("password generated", |
- form.type == PasswordForm::TYPE_GENERATED); |
- log.SetInteger("times used", form.times_used); |
- log.SetBoolean("use additional authentication", |
- form.use_additional_authentication); |
- log.SetBoolean("is PSL match", form.IsPublicSuffixMatch()); |
- LogValue(message, log); |
-} |
- |
-void SavePasswordProgressLogger::LogHTMLForm(const std::string& message, |
- const std::string& name_or_id, |
- const std::string& method, |
- const GURL& action) { |
+void SavePasswordProgressLogger::LogPasswordForm( |
+ SavePasswordProgressLogger::StringID label, |
+ const PasswordForm& form) { |
DictionaryValue log; |
- log.SetString("name_or_id", name_or_id); |
- log.SetString("method", method); |
- log.SetString("action", ScrubURL(action)); |
- LogValue(message, log); |
+ AddToDict(&log, STRING_SCHEME_MESSAGE, FormSchemeToStringID(form.scheme)); |
+ AddToDict(&log, STRING_SCHEME_MESSAGE, FormSchemeToStringID(form.scheme)); |
+ AddToDict(&log, STRING_SIGNON_REALM, GURL(form.signon_realm)); |
+ AddToDict( |
+ &log, STRING_ORIGINAL_SIGNON_REALM, GURL(form.original_signon_realm)); |
+ AddToDict(&log, STRING_ORIGIN, form.origin); |
+ AddToDict(&log, STRING_ACTION, form.action); |
+ AddToDict(&log, STRING_USERNAME_ELEMENT, form.username_element); |
+ AddToDict(&log, STRING_PASSWORD_ELEMENT, form.password_element); |
+ AddToDict( |
+ &log, STRING_PASSWORD_AUTOCOMPLETE_SET, form.password_autocomplete_set); |
+ AddToDict(&log, STRING_OLD_PASSWORD_ELEMENT, form.old_password_element); |
+ AddToDict(&log, STRING_SSL_VALID, form.ssl_valid); |
+ AddToDict(&log, |
+ STRING_PASSWORD_GENERATED, |
+ form.type == PasswordForm::TYPE_GENERATED); |
+ AddToDict(&log, STRING_TIMES_USED, form.times_used); |
+ AddToDict(&log, |
+ STRING_USE_ADDITIONAL_AUTHENTICATION, |
+ form.use_additional_authentication); |
+ AddToDict(&log, STRING_PSL_MATCH, form.IsPublicSuffixMatch()); |
+ LogValue(label, &log); |
} |
-void SavePasswordProgressLogger::LogURL(const std::string& message, |
- const GURL& url) { |
- LogValue(message, StringValue(ScrubURL(url))); |
+void SavePasswordProgressLogger::LogHTMLForm( |
+ SavePasswordProgressLogger::StringID label, |
+ const std::string& name_or_id, |
+ const std::string& method, |
+ const GURL& action) { |
+ DictionaryValue log; |
+ AddToDict(&log, STRING_NAME_OR_ID, name_or_id); |
+ AddToDict(&log, STRING_METHOD, FormMethodToStringID(method)); |
+ AddToDict(&log, STRING_ACTION, action); |
+ LogValue(label, &log); |
} |
-void SavePasswordProgressLogger::LogBoolean(const std::string& message, |
- bool value) { |
- LogValue(message, FundamentalValue(value)); |
+void SavePasswordProgressLogger::LogURL( |
+ SavePasswordProgressLogger::StringID label, |
+ const GURL& url) { |
+ LogValue(label, GetValueFromLog(url).get()); |
} |
-void SavePasswordProgressLogger::LogNumber(const std::string& message, |
- int value) { |
- LogValue(message, FundamentalValue(value)); |
+void SavePasswordProgressLogger::LogBoolean( |
+ SavePasswordProgressLogger::StringID label, |
+ bool truth_value) { |
+ LogValue(label, GetValueFromLog(truth_value).get()); |
} |
-void SavePasswordProgressLogger::LogNumber(const std::string& message, |
- size_t value) { |
- LogValue(message, FundamentalValue(checked_cast<int, size_t>(value))); |
+void SavePasswordProgressLogger::LogNumber( |
+ SavePasswordProgressLogger::StringID label, |
+ int signed_number) { |
+ LogValue(label, GetValueFromLog(signed_number).get()); |
} |
-void SavePasswordProgressLogger::LogFinalDecision(Decision decision) { |
- LogValue("Final decision taken", DecisionToStringValue(decision)); |
+void SavePasswordProgressLogger::LogNumber( |
+ SavePasswordProgressLogger::StringID label, |
+ size_t unsigned_number) { |
+ LogValue(label, |
+ GetValueFromLog(checked_cast<int, size_t>(unsigned_number)).get()); |
} |
-void SavePasswordProgressLogger::LogMessage(const std::string& message) { |
- LogValue("Message", StringValue(message)); |
+void SavePasswordProgressLogger::LogMessage( |
+ SavePasswordProgressLogger::StringID message) { |
+ LogValue(STRING_MESSAGE, GetValueFromLog(message).get()); |
} |
-void SavePasswordProgressLogger::LogValue(const std::string& name, |
- const Value& log) { |
+void SavePasswordProgressLogger::LogValue(StringID label, const Value* log) { |
std::string log_string; |
bool conversion_to_string_successful = base::JSONWriter::WriteWithOptions( |
- &log, base::JSONWriter::OPTIONS_PRETTY_PRINT, &log_string); |
+ log, base::JSONWriter::OPTIONS_PRETTY_PRINT, &log_string); |
DCHECK(conversion_to_string_successful); |
- SendLog(name + ": " + log_string); |
+ SendLog(GetStringFromID(label) + ": " + log_string); |
} |
} // namespace autofill |