[Autofill] Autofill fails to fill credit card number when split across fields.

components/autofill/core/browser/credit_card_field.cc

 // Copyright 2013 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 "components/autofill/core/browser/credit_card_field.h"
 
 #include <stddef.h>
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_regex_constants.h"
 #include "components/autofill/core/browser/autofill_scanner.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace autofill {
 
+// Credit card numbers are at most 19 digits in length.
+// [Ref: http://en.wikipedia.org/wiki/Bank_card_number]
+static const size_t kMaxValidCardNumberSize = 19;
+
 // static
 FormField* CreditCardField::Parse(AutofillScanner* scanner) {
   if (scanner->IsEnd())
     return NULL;
 
   scoped_ptr<CreditCardField> credit_card_field(new CreditCardField);
   size_t saved_cursor = scanner->SaveCursor();
 
   // Credit card fields can appear in many different orders.
   // We loop until no more credit card related fields are found, see |break| at
@@ skipping 19 matching lines @@
       }
 
       if (ParseField(scanner, name_pattern, &credit_card_field->cardholder_))
         continue;
 
       // As a hard-coded hack for Expedia's billing pages (expedia_checkout.html
       // and ExpediaBilling.html in our test suite), recognize separate fields
       // for the cardholder's first and last name if they have the labels "cfnm"
       // and "clnm".
       scanner->SaveCursor();
-      const AutofillField* first;
+      AutofillField* first;
       if (ParseField(scanner, base::ASCIIToUTF16("^cfnm"), &first) &&
           ParseField(scanner,
                      base::ASCIIToUTF16("^clnm"),
                      &credit_card_field->cardholder_last_)) {
         credit_card_field->cardholder_ = first;
         continue;
       }
       scanner->Rewind();
     }
 
@@ skipping 12 matching lines @@
     // has a plethora of names; we've seen "verification #",
     // "verification number", "card identification number" and others listed
     // in the |pattern| below.
     base::string16 pattern = base::UTF8ToUTF16(autofill::kCardCvcRe);
     if (!credit_card_field->verification_ &&
         ParseField(scanner, pattern, &credit_card_field->verification_)) {
       continue;
     }
 
     pattern = base::UTF8ToUTF16(autofill::kCardNumberRe);
-    if (!credit_card_field->number_ &&
-        ParseField(scanner, pattern, &credit_card_field->number_)) {
+    AutofillField* current_number_field;
+    if (ParseField(scanner, pattern, &current_number_field)) {
+      // Avoid autofilling any credit card number field having very low or high
+      // |start_index| on the HTML form.
+      size_t start_index = 0;
+      if (!credit_card_field->numbers_.empty()) {
+        size_t last_number_field_size =
+            credit_card_field->numbers_.back()->credit_card_number_offset() +
+            credit_card_field->numbers_.back()->max_length;
+
+        // In some cases, HTML form may have credit card number split across
+        // multiple input fields and either one or cumulatively having
+        // |max_length| more than |kMaxValidCardNumberSize|, mark these input
+        // form fields as invalid and skip autofilling them.
+        if (last_number_field_size == 0U ||
+            last_number_field_size >= kMaxValidCardNumberSize) {
+          // Mark that the credit card number splits are invalid. But keep
+          // scanning HTML form so that cursor moves beyond related fields.
+          credit_card_field->is_valid_card_number_split_ = false;

[Ilya Sherman, 2014/09/16 02:35:27]

I'm still not convinced that swallowing all of the credit card fields is
appropriate.  Even if we don't think that we understand the form well enough to
fill the credit card number, we can still fill the name and expiration date. 
Honestly, I think that a form like

<input id="CREDIT_CARD_NUMBER1" name="card_number1" maxlength=4 />
<input id="CREDIT_CARD_NUMBER2" name="card_number2" maxlength=40 />
<input id="CREDIT_CARD_NUMBER3" name="card_number3" maxlength=4 />
<input id="CREDIT_CARD_NUMBER4" name="card_number4" /> <br>

is implausible.  I could imagine encountering a form like

<input id="CREDIT_CARD_NUMBER" name="card_number1" />
<input id="NOT_A_CREDIT_CARD_NUMBER" name="card_number2" />

where our heuristics could incorrectly detect the second field as a credit card
number field even though it isn't.  I think the reasonable options are either
(1) to try to fill the first field(s) that we think are for the credit card
number, or (2) to swallow (i.e. refuse to fill) the credit card number fields,
but still be willing to fill the remaining credit card fields.  In either case,
I think that logic can be implemented entirely internally to this function, but
setting the contents of |credit_card_field->numbers_| appropriate.

[Pritam Nikam, 2014/09/16 10:49:36]

Done.

Going ahead with option (2) as you suggested i.e. to swallow (i.e. refuse to
fill) the credit card number fields, but still be willing to fill the remaining
credit card fields.

+        }
+
+        start_index = last_number_field_size;
+      }
+
+      current_number_field->set_credit_card_number_offset(start_index);
+      credit_card_field->numbers_.push_back(current_number_field);
       continue;
     }
 
     if (LowerCaseEqualsASCII(scanner->Cursor()->form_control_type, "month")) {
       credit_card_field->expiration_date_ = scanner->Cursor();
       scanner->Advance();
     } else {
       // First try to parse split month/year expiration fields.
       scanner->SaveCursor();
       pattern = base::UTF8ToUTF16(autofill::kExpirationMonthRe);
@@ skipping 62 matching lines @@
   if (credit_card_field->cardholder_)
     return credit_card_field.release();
 
   // On some pages, the user selects a card type using radio buttons
   // (e.g. test page Apple Store Billing.html).  We can't handle that yet,
   // so we treat the card type as optional for now.
   // The existence of a number or cvc in combination with expiration date is
   // a strong enough signal that this is a credit card.  It is possible that
   // the number and name were parsed in a separate part of the form.  So if
   // the cvc and date were found independently they are returned.
-  if ((credit_card_field->number_ || credit_card_field->verification_) &&
+  if ((!credit_card_field->numbers_.empty() ||
+       credit_card_field->verification_) &&
       (credit_card_field->expiration_date_ ||
        (credit_card_field->expiration_month_ &&
         credit_card_field->expiration_year_))) {
     return credit_card_field.release();
   }
 
   scanner->RewindTo(saved_cursor);
   return NULL;
 }
 
 CreditCardField::CreditCardField()
     : cardholder_(NULL),
       cardholder_last_(NULL),
       type_(NULL),
-      number_(NULL),
       verification_(NULL),
       expiration_month_(NULL),
       expiration_year_(NULL),
       expiration_date_(NULL),
-      exp_year_type_(CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR) {
+      exp_year_type_(CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR),
+      is_valid_card_number_split_(true) {
+}
+
+CreditCardField::~CreditCardField() {
 }
 
 bool CreditCardField::ClassifyField(ServerFieldTypeMap* map) const {
-  bool ok = AddClassification(number_, CREDIT_CARD_NUMBER, map);
+  // Bail-out autofilling for invalid credit card number splits.
+  if (!is_valid_card_number_split_)
+    return true;
+
+  bool ok = true;
+  for (size_t index = 0; index < numbers_.size(); ++index) {
+    ok = ok && AddClassification(numbers_.at(index), CREDIT_CARD_NUMBER, map);

[Ilya Sherman, 2014/09/16 02:35:27]

nit: Please use "numbers_[index]" rather than "numbers_.at(index)", as the
latter can throw an exception, whereas Chromium code does not handle exceptions.

[Pritam Nikam, 2014/09/16 10:49:36]

Done.

+  }
+
   ok = ok && AddClassification(type_, CREDIT_CARD_TYPE, map);
   ok = ok &&
        AddClassification(verification_, CREDIT_CARD_VERIFICATION_CODE, map);
 
   // If the heuristics detected first and last name in separate fields,
   // then ignore both fields. Putting them into separate fields is probably
   // wrong, because the credit card can also contain a middle name or middle
   // initial.
   if (cardholder_last_ == NULL)
     ok = ok && AddClassification(cardholder_, CREDIT_CARD_NAME, map);
@@ skipping 12 matching lines @@
 
 ServerFieldType CreditCardField::GetExpirationYearType() const {
   return (expiration_date_
               ? exp_year_type_
               : ((expiration_year_ && expiration_year_->max_length == 2)
                      ? CREDIT_CARD_EXP_2_DIGIT_YEAR
                      : CREDIT_CARD_EXP_4_DIGIT_YEAR));
 }
 
 }  // namespace autofill