Autofill should reject aggregated profiles where email address is found in non-email fields

chrome/browser/autofill/personal_data_manager.cc

 // Copyright (c) 2011 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 "chrome/browser/autofill/personal_data_manager.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/logging.h"
@@ skipping 84 matching lines @@
 // been met.  An address submitted via a form must have at least these fields
 // filled.  No verification of validity of the contents is preformed.  This is
 // and existence check only.
 bool IsMinimumAddress(const AutofillProfile& profile) {
   return !profile.GetInfo(ADDRESS_HOME_LINE1).empty() &&
          !profile.GetInfo(ADDRESS_HOME_CITY).empty() &&
          !profile.GetInfo(ADDRESS_HOME_STATE).empty() &&
          !profile.GetInfo(ADDRESS_HOME_ZIP).empty();
 }
 
+// Return true if the |field_type| and |value| are valid within the context
+// of importing a form.
+bool IsValidFieldTypeAndValue(const std::set<AutofillFieldType>& types_seen,
+                              AutofillFieldType field_type,
+                              const string16& value) {
+  // Abandon the import if two fields of the same type are encountered.
+  // This indicates ambiguous data or miscategorization of types.
+  // Make an exception for PHONE_HOME_NUMBER however as both prefix and
+  // suffix are stored against this type.
+  if (types_seen.count(field_type) &&
+      field_type != PHONE_HOME_NUMBER &&
+      field_type != PHONE_FAX_NUMBER) {
+    return false;
+  }
+
+  // Abandon the import if an email address value shows up in a field that is
+  // not an email address.
+  if (field_type != EMAIL_ADDRESS && IsValidEmail(value))
+    return false;

[Ilya Sherman, 2011/05/12 21:40:03]

It's a little confusing to me that this check is here, but the complementary
check is in IsValidLearnableProfile().  If you're not opposed, let's have them
live side by side -- either here or there is fine by me.

[dhollowa, 2011/05/12 23:24:12]

Done.

+
+  return true;
+}
+
 }  // namespace
 
 PersonalDataManager::~PersonalDataManager() {
   CancelPendingQuery(&pending_profiles_query_);
   CancelPendingQuery(&pending_creditcards_query_);
 }
 
 void PersonalDataManager::OnWebDataServiceRequestDone(
     WebDataService::Handle h,
     const WDTypedResult* result) {
@@ skipping 92 matching lines @@
     string16 value = CollapseWhitespace(field->value, false);
 
     // If we don't know the type of the field, or the user hasn't entered any
     // information into the field, then skip it.
     if (!field->IsFieldFillable() || value.empty())
       continue;
 
     AutofillFieldType field_type = field->type();
     FieldTypeGroup group(AutofillType(field_type).group());
 
-    // Abandon the import if two fields of the same type are encountered.
-    // This indicates ambiguous data or miscategorization of types.
-    // Make an exception for PHONE_HOME_NUMBER however as both prefix and
-    // suffix are stored against this type.
-    if (types_seen.count(field_type) &&
-        field_type != PHONE_HOME_NUMBER  &&
-        field_type != PHONE_FAX_NUMBER) {
+    // If the |field_type| and |value| don't pass basic validity checks then
+    // abandon the import.
+    if (!IsValidFieldTypeAndValue(types_seen, field_type, value)) {
       imported_profile.reset();
       local_imported_credit_card.reset();
       break;
-    } else {
-      types_seen.insert(field_type);
     }
 
+    types_seen.insert(field_type);
+
     if (group == AutofillType::CREDIT_CARD) {
       // If the user has a password set, we have no way of setting credit
       // card numbers.
-      if (!HasPassword()) {
-        if (LowerCaseEqualsASCII(field->form_control_type, "month")) {
-          DCHECK_EQ(CREDIT_CARD_EXP_MONTH, field_type);
-          local_imported_credit_card->SetInfoForMonthInputType(value);
-        } else {
-          if (field_type == CREDIT_CARD_NUMBER) {
-            // Clean up any imported credit card numbers.
-            value = CreditCard::StripSeparators(value);
-          }
-          local_imported_credit_card->SetInfo(field_type, value);
+      if (LowerCaseEqualsASCII(field->form_control_type, "month")) {
+        DCHECK_EQ(CREDIT_CARD_EXP_MONTH, field_type);
+        local_imported_credit_card->SetInfoForMonthInputType(value);
+      } else {
+        if (field_type == CREDIT_CARD_NUMBER) {
+          // Clean up any imported credit card numbers.
+          value = CreditCard::StripSeparators(value);
         }
-        ++importable_credit_card_fields;
+        local_imported_credit_card->SetInfo(field_type, value);
       }
+      ++importable_credit_card_fields;
     } else {
       // In the case of a phone number, if the whole phone number was entered
       // into a single field, then parse it and set the sub components.
       if (AutofillType(field_type).subgroup() ==
               AutofillType::PHONE_WHOLE_NUMBER) {
         string16 number;
         string16 city_code;
         string16 country_code;
         PhoneNumber::ParsePhoneNumber(value,
                                       &number,
@@ skipping 272 matching lines @@
        iter != profiles.end(); ++iter) {
     (*iter)->GetNonEmptyTypes(non_empty_types);
   }
 
   for (ScopedVector<CreditCard>::const_iterator iter = credit_cards_.begin();
        iter != credit_cards_.end(); ++iter) {
     (*iter)->GetNonEmptyTypes(non_empty_types);
   }
 }
 
-bool PersonalDataManager::HasPassword() {
-  return !password_hash_.empty();
-}
-
 bool PersonalDataManager::IsDataLoaded() const {
   return is_data_loaded_;
 }
 
 const std::vector<AutofillProfile*>& PersonalDataManager::profiles() const {
   // |profile_| is NULL in AutofillManagerTest.
   bool auxiliary_profiles_enabled = profile_ ? profile_->GetPrefs()->GetBoolean(
       prefs::kAutofillAuxiliaryProfilesEnabled) : false;
   if (!auxiliary_profiles_enabled)
     return web_profiles();
@@ skipping 414 matching lines @@
 }
 
 const AutofillMetrics* PersonalDataManager::metric_logger() const {
   return metric_logger_.get();
 }
 
 void PersonalDataManager::set_metric_logger(
     const AutofillMetrics* metric_logger) {
   metric_logger_.reset(metric_logger);
 }