Serialize form_data in Gnome keyring password store service.

Serializing form_data is required for server-side password generation heuristics. It was implemented on all others platforms before. Here it's implemented in gnome-keyring and libsecret backends (both of them works with Gnome keyring service).

BUG=463126

Review URL: https://codereview.chromium.org/980583002

Cr-Commit-Position: refs/heads/master@{#319628}

chrome/browser/password_manager/native_backend_gnome_x.cc

@@ -158,7 +158,8 @@ scoped_ptr<PasswordForm> FormFromAttributes(GnomeKeyringAttributeList* attrs) {
   form->generation_upload_status =
       static_cast<PasswordForm::GenerationUploadStatus>(
           uint_attr_map["generation_upload_status"]);
-
+  DeserializeFormDataFromBase64String(string_attr_map["form_data"],
+      &form->form_data);
   return form.Pass();
 }
 
@@ -220,11 +221,6 @@ void ConvertFormList(GList* found,
 }
 
 // Schema is analagous to the fields in PasswordForm.
-// TODO(gcasto): Adding 'form_data' would be nice, but we would need to
-// serialize in a way that is guaranteed to not have any embedded NULLs. Pickle
-// doesn't make this guarantee, so we just don't serialize this field. Since
-// it's only used to crowd source data collection it doesn't matter that much
-// if it's not available on this platform.
 const GnomeKeyringPasswordSchema kGnomeSchema = {
   GNOME_KEYRING_ITEM_GENERIC_SECRET, {
     { "origin_url", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
@@ -247,6 +243,7 @@ const GnomeKeyringPasswordSchema kGnomeSchema = {
     { "federation_url", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
     { "skip_zero_click", GNOME_KEYRING_ATTRIBUTE_TYPE_UINT32 },
     { "generation_upload_status", GNOME_KEYRING_ATTRIBUTE_TYPE_UINT32 },
+    { "form_data", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
     // This field is always "chrome" so that we can search for it.
     { "application", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
     { nullptr }
@@ -339,6 +336,8 @@ void GKRMethod::AddLogin(const PasswordForm& form, const char* app_string) {
   if (!date_created)
     date_created = base::Time::Now().ToInternalValue();
   int64 date_synced = form.date_synced.ToInternalValue();
+  std::string form_data;
+  SerializeFormDataToBase64String(form.form_data, &form_data);
   gnome_keyring_store_password(
       &kGnomeSchema,
       nullptr,  // Default keyring.
@@ -367,6 +366,7 @@ void GKRMethod::AddLogin(const PasswordForm& form, const char* app_string) {
       "federation_url", form.federation_url.spec().c_str(),
       "skip_zero_click", form.skip_zero_click,
       "generation_upload_status", form.generation_upload_status,
+      "form_data", form_data.c_str(),
       "application", app_string,
       nullptr);
 }

chrome/browser/password_manager/native_backend_gnome_x_unittest.cc

@@ -359,6 +359,8 @@ class NativeBackendGnomeTest : public testing::Test {
     form_google_.federation_url = GURL("http://www.google.com/federation_url");
     form_google_.skip_zero_click = true;
     form_google_.generation_upload_status = PasswordForm::POSITIVE_SIGNAL_SENT;
+    form_google_.form_data.name = UTF8ToUTF16("form_name");
+    form_google_.form_data.user_submitted = true;
 
     form_facebook_.origin = GURL("http://www.facebook.com/");
     form_facebook_.action = GURL("http://www.facebook.com/login");
@@ -448,7 +450,7 @@ class NativeBackendGnomeTest : public testing::Test {
     EXPECT_EQ("login", item->keyring);
     EXPECT_EQ(form.origin.spec(), item->display_name);
     EXPECT_EQ(UTF16ToUTF8(form.password_value), item->password);
-    EXPECT_EQ(21u, item->attributes.size());
+    EXPECT_EQ(22u, item->attributes.size());
     CheckStringAttribute(item, "origin_url", form.origin.spec());
     CheckStringAttribute(item, "action_url", form.action.spec());
     CheckStringAttribute(item, "username_element",
@@ -476,6 +478,10 @@ class NativeBackendGnomeTest : public testing::Test {
     CheckUint32Attribute(item, "generation_upload_status",
                          form.generation_upload_status);
     CheckStringAttribute(item, "application", app_string);
+    autofill::FormData actual;
+    DeserializeFormDataFromBase64String(
+        item->attributes.at("form_data").value_string, &actual);
+    EXPECT_TRUE(form.form_data.SameFormAs(actual));
   }
 
   // Saves |credentials| and then gets logins matching |url| and |scheme|.
@@ -772,7 +778,7 @@ TEST_F(NativeBackendGnomeTest, BasicListLogins) {
 
   BrowserThread::PostTask(
       BrowserThread::DB, FROM_HERE,
-      base::Bind(base::IgnoreResult( &NativeBackendGnome::AddLogin),
+      base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin),
                  base::Unretained(&backend), form_google_));
 
   ScopedVector<autofill::PasswordForm> form_list;

chrome/browser/password_manager/native_backend_libsecret.cc

@@ -91,11 +91,6 @@ namespace {
 const char kLibsecretAppString[] = "chrome";
 
 // Schema is analagous to the fields in PasswordForm.
-// TODO(gcasto): Adding 'form_data' would be nice, but we would need to
-// serialize in a way that is guaranteed to not have any embedded NULLs. Pickle
-// doesn't make this guarantee, so we just don't serialize this field. Since
-// it's only used to crowd source data collection it doesn't matter that much
-// if it's not available on this platform.
 const SecretSchema kLibsecretSchema = {
     "chrome_libsecret_password_schema",
     // We have to use SECRET_SCHEMA_DONT_MATCH_NAME in order to get old
@@ -121,6 +116,7 @@ const SecretSchema kLibsecretSchema = {
      {"federation_url", SECRET_SCHEMA_ATTRIBUTE_STRING},
      {"skip_zero_click", SECRET_SCHEMA_ATTRIBUTE_INTEGER},
      {"generation_upload_status", SECRET_SCHEMA_ATTRIBUTE_INTEGER},
+     {"form_data", SECRET_SCHEMA_ATTRIBUTE_STRING},
      // This field is always "chrome-profile_id" so that we can search for it.
      {"application", SECRET_SCHEMA_ATTRIBUTE_STRING},
      {nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING}}};
@@ -194,7 +190,8 @@ scoped_ptr<PasswordForm> FormOutOfAttributes(GHashTable* attrs) {
   form->generation_upload_status =
       static_cast<PasswordForm::GenerationUploadStatus>(
           GetUintFromAttributes(attrs, "generation_upload_status"));
-
+  DeserializeFormDataFromBase64String(
+      GetStringFromAttributes(attrs, "form_data"), &form->form_data);
   return form.Pass();
 }
 
@@ -416,6 +413,8 @@ bool NativeBackendLibsecret::RawAddLogin(const PasswordForm& form) {
   if (!date_created)
     date_created = base::Time::Now().ToInternalValue();
   int64 date_synced = form.date_synced.ToInternalValue();
+  std::string form_data;
+  SerializeFormDataToBase64String(form.form_data, &form_data);
   GError* error = nullptr;
   secret_password_store_sync(
       &kLibsecretSchema,
@@ -444,6 +443,7 @@ bool NativeBackendLibsecret::RawAddLogin(const PasswordForm& form) {
       "federation_url", form.federation_url.spec().c_str(),
       "skip_zero_click", form.skip_zero_click,
       "generation_upload_status", form.generation_upload_status,
+      "form_data", form_data.c_str(),
       "application", app_string_.c_str(), nullptr);
 
   if (error) {

chrome/browser/password_manager/native_backend_libsecret_unittest.cc

@@ -281,6 +281,8 @@ class NativeBackendLibsecretTest : public testing::Test {
     form_google_.federation_url = GURL("http://www.google.com/federation_url");
     form_google_.skip_zero_click = true;
     form_google_.generation_upload_status = PasswordForm::POSITIVE_SIGNAL_SENT;
+    form_google_.form_data.name = UTF8ToUTF16("form_name");
+    form_google_.form_data.user_submitted = true;
 
     form_facebook_.origin = GURL("http://www.facebook.com/");
     form_facebook_.action = GURL("http://www.facebook.com/login");
@@ -356,7 +358,7 @@ class NativeBackendLibsecretTest : public testing::Test {
                            const PasswordForm& form,
                            const std::string& app_string) {
     EXPECT_EQ(UTF16ToUTF8(form.password_value), item->value->password);
-    EXPECT_EQ(21u, g_hash_table_size(item->attributes));
+    EXPECT_EQ(22u, g_hash_table_size(item->attributes));
     CheckStringAttribute(item, "origin_url", form.origin.spec());
     CheckStringAttribute(item, "action_url", form.action.spec());
     CheckStringAttribute(item, "username_element",
@@ -385,6 +387,11 @@ class NativeBackendLibsecretTest : public testing::Test {
     CheckUint32Attribute(item, "generation_upload_status",
                          form.generation_upload_status);
     CheckStringAttribute(item, "application", app_string);
+    autofill::FormData actual;
+    DeserializeFormDataFromBase64String(
+        static_cast<char*>(g_hash_table_lookup(item->attributes, "form_data")),
+        &actual);
+    EXPECT_TRUE(form.form_data.SameFormAs(actual));
   }
 
   // Saves |credentials| and then gets logins matching |url| and |scheme|.

components/autofill/core/common/form_data.cc

@@ -4,6 +4,7 @@
 
 #include "components/autofill/core/common/form_data.h"
 
+#include "base/base64.h"
 #include "base/pickle.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -124,6 +125,15 @@ void SerializeFormData(const FormData& form_data, Pickle* pickle) {
   pickle->WriteBool(form_data.is_form_tag);
 }
 
+void SerializeFormDataToBase64String(const FormData& form_data,
+                                     std::string* output) {
+  Pickle pickle;
+  SerializeFormData(form_data, &pickle);
+  Base64Encode(
+      base::StringPiece(static_cast<const char*>(pickle.data()), pickle.size()),
+      output);
+}
+
 bool DeserializeFormData(PickleIterator* iter, FormData* form_data) {
   int version;
   if (!iter->ReadInt(&version)) {
@@ -169,4 +179,15 @@ bool DeserializeFormData(PickleIterator* iter, FormData* form_data) {
   return true;
 }
 
+bool DeserializeFormDataFromBase64String(const base::StringPiece& input,
+                                         FormData* form_data) {
+  if (input.empty())
+    return false;
+  std::string pickle_data;
+  Base64Decode(input, &pickle_data);
+  Pickle pickle(pickle_data.data(), static_cast<int>(pickle_data.size()));
+  PickleIterator iter(pickle);
+  return DeserializeFormData(&iter, form_data);
+}
+
 }  // namespace autofill

components/autofill/core/common/form_data.h

@@ -44,12 +44,21 @@ struct FormData {
 std::ostream& operator<<(std::ostream& os, const FormData& form);
 
 // Serialize FormData. Used by the PasswordManager to persist FormData
-// pertaining to password forms. Serialized data is appended to |pickle|
+// pertaining to password forms. Serialized data is appended to |pickle|.
 void SerializeFormData(const FormData& form_data, Pickle* pickle);
 // Deserialize FormData. This assumes that |iter| is currently pointing to
 // the part of a pickle created by SerializeFormData. Returns true on success.
 bool DeserializeFormData(PickleIterator* iter, FormData* form_data);
 
+// Serialize FormData. Used by the PasswordManager to persist FormData
+// pertaining to password forms in base64 string. It is useful since in some
+// cases we need to store C strings without embedded '\0' symbols.
+void SerializeFormDataToBase64String(const FormData& form_data,
+                                     std::string* output);
+// Deserialize FormData. Returns true on success.
+bool DeserializeFormDataFromBase64String(const base::StringPiece& input,
+                                         FormData* form_data);
+
 }  // namespace autofill
 
 #endif  // COMPONENTS_AUTOFILL_CORE_COMMON_FORM_DATA_H_

components/autofill/core/common/form_data_unittest.cc

@@ -147,4 +147,18 @@ TEST(FormDataTest, SerializeIncorrectFormatAndDeserialize) {
   EXPECT_FALSE(DeserializeFormData(&iter, &actual));
 }
 
+TEST(FormDataTest, SerializeAndDeserializeInStrings) {
+  FormData data;
+  FillInDummyFormData(&data);
+  data.is_form_tag = false;
+
+  std::string serialized_data;
+  SerializeFormDataToBase64String(data, &serialized_data);
+
+  FormData actual;
+  EXPECT_TRUE(DeserializeFormDataFromBase64String(serialized_data, &actual));
+
+  EXPECT_TRUE(actual.SameFormAs(data));
+}
+
 }  // namespace autofill