[Autofill] Boundary check on repeated fields rationalization.

The case where the first field was rationalized would crash.

If the first field is rationalized, and we want to apply the
rationalization to the fields before that, we would end up at
the biggest unsigned int (since it's a size_t), which would
cause a crash.
Solution: If the first field is rationalized, we don't need
to apply the rationalization to the fields before that obviously.
Just return.
Test added.

Bug: 856962,857011
Change-Id: I0f4471e57fa0fa8bae9b7345ce774122f25e3410
Reviewed-on: https://chromium-review.googlesource.com/1117131
Commit-Queue: Parastoo Geranmayeh <parastoog@google.com>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570828}

components/autofill/core/browser/form_structure.cc

@@ -1320,26 +1320,30 @@ void FormStructure::ApplyRationalizationsToHiddenSelects(
     ServerFieldType new_type) {
   ServerFieldType old_type = fields_[field_index]->Type().GetStorableType();
 
-  // Walk on the hidden select fields right before the field_index which share
+  // Walk on the hidden select fields right after the field_index which share
   // the same type with the field_index, and apply the rationalization to them
   // as well. These fields, if any, function as one field with the field_index.
-  for (auto current_index = field_index - 1;; current_index--) {
+  for (auto current_index = field_index + 1; current_index < fields_.size();
+       current_index++) {
     if (fields_[current_index]->IsVisible() ||
         fields_[current_index]->form_control_type != "select-one" ||
         fields_[current_index]->Type().GetStorableType() != old_type)
       break;
     fields_[current_index]->SetTypeTo(AutofillType(new_type));
-    if (current_index == 0)
-      break;
   }
-  // Same for the fields coming right after the field_index.
-  for (auto current_index = field_index + 1; current_index < fields_.size();
-       current_index++) {
+
+  // Same for the fields coming right before the field_index. (No need to check
+  // for the fields appearing before the first field!)
+  if (field_index == 0)
+    return;
+  for (auto current_index = field_index - 1;; current_index--) {
     if (fields_[current_index]->IsVisible() ||
         fields_[current_index]->form_control_type != "select-one" ||
         fields_[current_index]->Type().GetStorableType() != old_type)
       break;
     fields_[current_index]->SetTypeTo(AutofillType(new_type));
+    if (current_index == 0)
+      break;
   }
 }
 

components/autofill/core/browser/form_structure_unittest.cc

@@ -5662,4 +5662,142 @@ TEST_F(FormStructureTest,
             forms[0]->field(13)->Type().GetStorableType());
 }
 
+TEST_F(FormStructureTest, RationalizeRepreatedFields_FirstFieldRationalized) {
+  base::test::ScopedFeatureList feature_list;
+  InitFeature(&feature_list, kAutofillRationalizeRepeatedServerPredictions,
+              true);
+
+  FormData form;
+  form.origin = GURL("http://foo.com");
+  FormFieldData field;
+  field.form_control_type = "text";
+  field.max_length = 10000;
+
+  field.section = "billing";
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country");
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country2");
+  field.form_control_type = "select-one";
+  field.is_focusable = false;  // hidden
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country3");
+  field.form_control_type = "select-one";
+  field.is_focusable = false;  // hidden
+  form.fields.push_back(field);
+
+  field.is_focusable = true;  // visible
+
+  field.label = ASCIIToUTF16("Full Name");
+  field.name = ASCIIToUTF16("fullName");
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("State");
+  field.name = ASCIIToUTF16("state");
+  form.fields.push_back(field);
+
+  AutofillQueryResponseContents response;
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_STATE);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_STATE);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_STATE);
+  response.add_field()->set_overall_type_prediction(NAME_FULL);
+  response.add_field()->set_overall_type_prediction(ADDRESS_BILLING_STATE);
+
+  std::string response_string;
+  ASSERT_TRUE(response.SerializeToString(&response_string));
+
+  FormStructure form_structure(form);
+  std::vector<FormStructure*> forms;
+  forms.push_back(&form_structure);
+
+  // Will call RationalizeFieldTypePredictions
+  FormStructure::ParseQueryResponse(response_string, forms);
+
+  ASSERT_EQ(1U, forms.size());
+  ASSERT_EQ(5U, forms[0]->field_count());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(0)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(1)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(2)->Type().GetStorableType());
+  EXPECT_EQ(NAME_FULL, forms[0]->field(3)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_STATE, forms[0]->field(4)->Type().GetStorableType());
+}
+
+TEST_F(FormStructureTest, RationalizeRepreatedFields_LastFieldRationalized) {
+  base::test::ScopedFeatureList feature_list;
+  InitFeature(&feature_list, kAutofillRationalizeRepeatedServerPredictions,
+              true);
+
+  FormData form;
+  form.origin = GURL("http://foo.com");
+  FormFieldData field;
+  field.form_control_type = "text";
+  field.max_length = 10000;
+
+  field.section = "billing";
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country");
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country2");
+  field.form_control_type = "select-one";
+  field.is_focusable = false;  // hidden
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("Country");
+  field.name = ASCIIToUTF16("country3");
+  field.form_control_type = "select-one";
+  field.is_focusable = false;  // hidden
+  form.fields.push_back(field);
+
+  field.is_focusable = true;  // visible
+
+  field.label = ASCIIToUTF16("Full Name");
+  field.name = ASCIIToUTF16("fullName");
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("State");
+  field.name = ASCIIToUTF16("state");
+  field.is_focusable = false;  // hidden
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("State");
+  field.name = ASCIIToUTF16("state2");
+  field.is_focusable = true;  // visible
+  form.fields.push_back(field);
+
+  AutofillQueryResponseContents response;
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_COUNTRY);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_COUNTRY);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_COUNTRY);
+  response.add_field()->set_overall_type_prediction(NAME_FULL);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_COUNTRY);
+  response.add_field()->set_overall_type_prediction(ADDRESS_HOME_COUNTRY);
+
+  std::string response_string;
+  ASSERT_TRUE(response.SerializeToString(&response_string));
+
+  FormStructure form_structure(form);
+  std::vector<FormStructure*> forms;
+  forms.push_back(&form_structure);
+
+  // Will call RationalizeFieldTypePredictions
+  FormStructure::ParseQueryResponse(response_string, forms);
+
+  ASSERT_EQ(1U, forms.size());
+  ASSERT_EQ(6U, forms[0]->field_count());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(0)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(1)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_COUNTRY, forms[0]->field(2)->Type().GetStorableType());
+  EXPECT_EQ(NAME_FULL, forms[0]->field(3)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_STATE, forms[0]->field(4)->Type().GetStorableType());
+  EXPECT_EQ(ADDRESS_HOME_STATE, forms[0]->field(5)->Type().GetStorableType());
+}
+
 }  // namespace autofill