diff options
author | engedy@chromium.org <engedy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-10 16:41:44 +0000 |
---|---|---|
committer | engedy@chromium.org <engedy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-10 16:41:44 +0000 |
commit | 006888751987877dfad35967348a168a419050cb (patch) | |
tree | 4a67ef55cb5760ff30e2049148e0346a38c1acc3 | |
parent | 8a3036fb8a891b9d8104fd0fadee418d9bca3681 (diff) | |
download | chromium_src-006888751987877dfad35967348a168a419050cb.zip chromium_src-006888751987877dfad35967348a168a419050cb.tar.gz chromium_src-006888751987877dfad35967348a168a419050cb.tar.bz2 |
Implement autocomplete='current-password' and 'new-password'.
BUG=375333
# Depends on recently landed CL, ran try bots with manually specified base revison.
NOTRY=True
Review URL: https://codereview.chromium.org/369323003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282356 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | components/autofill/content/renderer/password_form_conversion_utils.cc | 41 | ||||
-rw-r--r-- | components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc | 173 |
2 files changed, 189 insertions, 25 deletions
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc index fb0a699..6b4b780 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils.cc +++ b/components/autofill/content/renderer/password_form_conversion_utils.cc @@ -28,21 +28,46 @@ static const size_t kMaxPasswords = 3; // Checks in a case-insensitive way if the autocomplete attribute for the given // |element| is present and has the specified |value_in_lowercase|. -bool HasAutocompleteAttributeValue(const WebInputElement* element, +bool HasAutocompleteAttributeValue(const WebInputElement& element, const char* value_in_lowercase) { - return LowerCaseEqualsASCII(element->getAttribute("autocomplete"), + return LowerCaseEqualsASCII(element.getAttribute("autocomplete"), value_in_lowercase); } // Helper to determine which password is the main (current) one, and which is // the new password (e.g., on a sign-up or change password form), if any. bool LocateSpecificPasswords(std::vector<WebInputElement> passwords, - WebInputElement* password, + WebInputElement* current_password, WebInputElement* new_password) { + DCHECK(current_password && current_password->isNull()); + DCHECK(new_password && new_password->isNull()); + + // First, look for elements marked with either autocomplete='current-password' + // or 'new-password' -- if we find any, take the hint, and treat the first of + // each kind as the element we are looking for. + for (std::vector<WebInputElement>::const_iterator it = passwords.begin(); + it != passwords.end(); it++) { + if (HasAutocompleteAttributeValue(*it, "current-password") && + current_password->isNull()) { + *current_password = *it; + } else if (HasAutocompleteAttributeValue(*it, "new-password") && + new_password->isNull()) { + *new_password = *it; + } + } + + // If we have seen an element with either of autocomplete attributes above, + // take that as a signal that the page author must have intentionally left the + // rest of the password fields unmarked. Perhaps they are used for other + // purposes, e.g., PINs, OTPs, and the like. So we skip all the heuristics we + // normally do, and ignore the rest of the password fields. + if (!current_password->isNull() || !new_password->isNull()) + return true; + switch (passwords.size()) { case 1: // Single password, easy. - *password = passwords[0]; + *current_password = passwords[0]; break; case 2: if (passwords[0].value() == passwords[1].value()) { @@ -52,7 +77,7 @@ bool LocateSpecificPasswords(std::vector<WebInputElement> passwords, *new_password = passwords[0]; } else { // Assume first is old password, second is new (no choice but to guess). - *password = passwords[0]; + *current_password = passwords[0]; *new_password = passwords[1]; } break; @@ -66,12 +91,12 @@ bool LocateSpecificPasswords(std::vector<WebInputElement> passwords, } else if (passwords[1].value() == passwords[2].value()) { // New password is the duplicated one, and comes second; or empty form // with 3 password fields, in which case we will assume this layout. - *password = passwords[0]; + *current_password = passwords[0]; *new_password = passwords[1]; } else if (passwords[0].value() == passwords[1].value()) { // It is strange that the new password comes first, but trust more which // fields are duplicated than the ordering of fields. - *password = passwords[2]; + *current_password = passwords[2]; *new_password = passwords[0]; } else { // Three different passwords, or first and last match with middle @@ -128,7 +153,7 @@ void GetPasswordForm(const WebFormElement& form, PasswordForm* password_form) { // Various input types such as text, url, email can be a username field. if (input_element->isTextField() && !input_element->isPasswordField()) { - if (HasAutocompleteAttributeValue(input_element, "username")) { + if (HasAutocompleteAttributeValue(*input_element, "username")) { if (has_seen_element_with_autocomplete_username_before) { // A second or subsequent element marked with autocomplete='username'. // This makes us less confident that we have understood the form. We diff --git a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc index c38ff86..a7e31dc 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc +++ b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc @@ -39,11 +39,11 @@ class PasswordFormBuilder { // |name_and_id|, |value|, and |autocomplete| attributes. The |autocomplete| // argument can take two special values, namely: // 1.) NULL, causing no autocomplete attribute to be added, - // 2.) "", causing an empty attribute (i.e. autocomplete='') to be added. + // 2.) "", causing an empty attribute (i.e. autocomplete="") to be added. void AddUsernameField(const char* name_and_id, const char* value, const char* autocomplete) { - std::string autocomplete_attribute(autocomplete != NULL ? + std::string autocomplete_attribute(autocomplete ? base::StringPrintf("autocomplete=\"%s\"", autocomplete) : ""); base::StringAppendF( &html_, @@ -52,12 +52,17 @@ class PasswordFormBuilder { } // Appends a new password-type field at the end of the form, having the - // specified |name_and_id| and |value| attributes. - void AddPasswordField(const char* name_and_id, const char* value) { + // specified |name_and_id|, |value|, and |autocomplete| attributes. Special + // values for |autocomplete| are the same as in AddUsernameField. + void AddPasswordField(const char* name_and_id, + const char* value, + const char* autocomplete) { + std::string autocomplete_attribute(autocomplete ? + base::StringPrintf("autocomplete=\"%s\"", autocomplete): ""); base::StringAppendF( &html_, - "<INPUT type=\"password\" name=\"%s\" id=\"%s\" value=\"%s\"/>", - name_and_id, name_and_id, value); + "<INPUT type=\"password\" name=\"%s\" id=\"%s\" value=\"%s\" %s/>", + name_and_id, name_and_id, value, autocomplete_attribute.c_str()); } // Appends a new submit-type field at the end of the form. @@ -111,7 +116,7 @@ TEST_F(PasswordFormConversionUtilsTest, ValidWebFormElementToPasswordForm) { PasswordFormBuilder builder(kTestFormActionURL); builder.AddUsernameField("username", "johnsmith", NULL); builder.AddSubmitButton(); - builder.AddPasswordField("password", "secret"); + builder.AddPasswordField("password", "secret", NULL); std::string html = builder.ProduceHTML(); scoped_ptr<PasswordForm> password_form; @@ -135,7 +140,7 @@ TEST_F(PasswordFormConversionUtilsTest, InvalidWebFormElementToPasswordForm) { PasswordFormBuilder builder("invalid_target"); builder.AddUsernameField("username", "johnsmith", NULL); builder.AddSubmitButton(); - builder.AddPasswordField("password", "secret"); + builder.AddPasswordField("password", "secret", NULL); std::string html = builder.ProduceHTML(); scoped_ptr<PasswordForm> password_form; @@ -147,11 +152,11 @@ TEST_F(PasswordFormConversionUtilsTest, WebFormWithMultipleUseNameAndPassWordFieldsToPasswordForm) { PasswordFormBuilder builder(kTestFormActionURL); builder.AddUsernameField("username1", "John", NULL); - builder.AddPasswordField("password1", "oldsecret"); + builder.AddPasswordField("password1", "oldsecret", NULL); builder.AddUsernameField("username2", "William", NULL); - builder.AddPasswordField("password2", "secret"); + builder.AddPasswordField("password2", "secret", NULL); builder.AddUsernameField("username3", "Smith", NULL); - builder.AddPasswordField("password3", "secret"); + builder.AddPasswordField("password3", "secret", NULL); builder.AddSubmitButton(); std::string html = builder.ProduceHTML(); @@ -185,9 +190,9 @@ TEST_F(PasswordFormConversionUtilsTest, WebFormwithThreeDifferentPasswordsToPasswordForm) { PasswordFormBuilder builder(kTestFormActionURL); builder.AddUsernameField("username1", "John", NULL); - builder.AddPasswordField("password1", "alpha"); - builder.AddPasswordField("password2", "beta"); - builder.AddPasswordField("password3", "gamma"); + builder.AddPasswordField("password1", "alpha", NULL); + builder.AddPasswordField("password2", "beta", NULL); + builder.AddPasswordField("password3", "gamma", NULL); builder.AddSubmitButton(); std::string html = builder.ProduceHTML(); @@ -218,12 +223,12 @@ TEST_F(PasswordFormConversionUtilsTest, {{"username", NULL, "username"}, "username1", "John", "Smith"}, {{"username", "username", "username"}, "username1", "John", "William+Smith"}, - // When there is an empty autocomplete attribute (i.e. autocomplete=''), + // When there is an empty autocomplete attribute (i.e. autocomplete=""), // it should have the same effect as having no attribute whatsoever. {{"", "", ""}, "username2", "William", "John+Smith"}, {{"", "", "username"}, "username3", "Smith", ""}, {{"username", "", "username"}, "username1", "John", "Smith"}, - // Whether attribute values are upper or mixed case, it should not matter. + // It should not matter if attribute values are upper or mixed case. {{"USERNAME", NULL, "uSeRNaMe"}, "username1", "John", "Smith"}, {{"uSeRNaMe", NULL, "USERNAME"}, "username1", "John", "Smith"}}; @@ -233,7 +238,7 @@ TEST_F(PasswordFormConversionUtilsTest, PasswordFormBuilder builder(kTestFormActionURL); builder.AddUsernameField("username1", "John", cases[i].autocomplete[0]); builder.AddUsernameField("username2", "William", cases[i].autocomplete[1]); - builder.AddPasswordField("password", "secret"); + builder.AddPasswordField("password", "secret", NULL); builder.AddUsernameField("username3", "Smith", cases[i].autocomplete[2]); builder.AddSubmitButton(); std::string html = builder.ProduceHTML(); @@ -253,4 +258,138 @@ TEST_F(PasswordFormConversionUtilsTest, } } +TEST_F(PasswordFormConversionUtilsTest, + PasswordFieldsWithAutocompleteAttributes) { + // Each test case consists of a set of parameters to be plugged into the + // PasswordFormBuilder below, plus the corresponding expectations. + struct TestCase { + const char* autocomplete[3]; + const char* expected_password_element; + const char* expected_password_value; + const char* expected_new_password_element; + const char* expected_new_password_value; + } cases[] = { + // When there are elements marked with autocomplete='current-password', + // but no elements with 'new-password', we should treat the first of the + // former kind as the current password, and ignore all other password + // fields, assuming they are not intentionally not marked. They might be + // for other purposes, such as PINs, OTPs, and the like. Actual values in + // the password fields should be ignored in all cases below. + {{"current-password", NULL, NULL}, + "password1", "alpha", "", ""}, + {{NULL, "current-password", NULL}, + "password2", "beta", "", ""}, + {{NULL, NULL, "current-password"}, + "password3", "gamma", "", ""}, + {{NULL, "current-password", "current-password"}, + "password2", "beta", "", ""}, + {{"current-password", NULL, "current-password"}, + "password1", "alpha", "", ""}, + {{"current-password", "current-password", NULL}, + "password1", "alpha", "", ""}, + {{"current-password", "current-password", "current-password"}, + "password1", "alpha", "", ""}, + // The same goes vice versa for autocomplete='new-password'. + {{"new-password", NULL, NULL}, + "", "", "password1", "alpha"}, + {{NULL, "new-password", NULL}, + "", "", "password2", "beta"}, + {{NULL, NULL, "new-password"}, + "", "", "password3", "gamma"}, + {{NULL, "new-password", "new-password"}, + "", "", "password2", "beta"}, + {{"new-password", NULL, "new-password"}, + "", "", "password1", "alpha"}, + {{"new-password", "new-password", NULL}, + "", "", "password1", "alpha"}, + {{"new-password", "new-password", "new-password"}, + "", "", "password1", "alpha"}, + // When there is one element marked with autocomplete='current-password', + // and one with 'new-password', just comply, regardless of their order. + // Ignore the unmarked password field(s) for the same reason as above. + {{"current-password", "new-password", NULL}, + "password1", "alpha", "password2", "beta"}, + {{"current-password", NULL, "new-password"}, + "password1", "alpha", "password3", "gamma"}, + {{NULL, "current-password", "new-password"}, + "password2", "beta", "password3", "gamma"}, + {{"new-password", "current-password", NULL}, + "password2", "beta", "password1", "alpha"}, + {{"new-password", NULL, "current-password"}, + "password3", "gamma", "password1", "alpha"}, + {{NULL, "new-password", "current-password"}, + "password3", "gamma", "password2", "beta"}, + // In case of duplicated elements of either kind, go with the first one of + // its kind. + {{"current-password", "current-password", "new-password"}, + "password1", "alpha", "password3", "gamma"}, + {{"current-password", "new-password", "current-password"}, + "password1", "alpha", "password2", "beta"}, + {{"new-password", "current-password", "current-password"}, + "password2", "beta", "password1", "alpha"}, + {{"current-password", "new-password", "new-password"}, + "password1", "alpha", "password2", "beta"}, + {{"new-password", "current-password", "new-password"}, + "password2", "beta", "password1", "alpha"}, + {{"new-password", "new-password", "current-password"}, + "password3", "gamma", "password1", "alpha"}, + // When there is an empty autocomplete attribute (i.e. autocomplete=""), + // it should have the same effect as having no attribute whatsoever. + {{"current-password", "", ""}, + "password1", "alpha", "", ""}, + {{"", "", "new-password"}, + "", "", "password3", "gamma"}, + {{"", "new-password", ""}, + "", "", "password2", "beta"}, + {{"", "current-password", "current-password"}, + "password2", "beta", "", ""}, + {{"new-password", "", "new-password"}, + "", "", "password1", "alpha"}, + {{"new-password", "", "current-password"}, + "password3", "gamma", "password1", "alpha"}, + // It should not matter if attribute values are upper or mixed case. + {{NULL, "current-password", NULL}, + "password2", "beta", "", ""}, + {{NULL, "CURRENT-PASSWORD", NULL}, + "password2", "beta", "", ""}, + {{NULL, "new-password", NULL}, + "", "", "password2", "beta"}, + {{NULL, "nEw-PaSsWoRd", NULL}, + "", "", "password2", "beta"}}; + + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) { + SCOPED_TRACE(testing::Message() << "Iteration " << i); + + PasswordFormBuilder builder(kTestFormActionURL); + builder.AddPasswordField("password1", "alpha", cases[i].autocomplete[0]); + builder.AddUsernameField("username1", "William", NULL); + builder.AddPasswordField("password2", "beta", cases[i].autocomplete[1]); + builder.AddUsernameField("username2", "Smith", NULL); + builder.AddPasswordField("password3", "gamma", cases[i].autocomplete[2]); + builder.AddSubmitButton(); + std::string html = builder.ProduceHTML(); + + scoped_ptr<PasswordForm> password_form; + ASSERT_NO_FATAL_FAILURE(LoadHTMLAndConvertForm(html, &password_form)); + ASSERT_NE(static_cast<PasswordForm*>(NULL), password_form.get()); + + // Any constellation of password autocomplete attributes should have no + // effect on that the first text-type input field before a password field + // should be selected as the username. + EXPECT_EQ(base::UTF8ToUTF16("username1"), password_form->username_element); + EXPECT_EQ(base::UTF8ToUTF16("William"), password_form->username_value); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_password_element), + password_form->password_element); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_password_value), + password_form->password_value); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_element), + password_form->new_password_element); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_value), + password_form->new_password_value); + ASSERT_EQ(1u, password_form->other_possible_usernames.size()); + EXPECT_EQ(base::UTF8ToUTF16("Smith"), + password_form->other_possible_usernames[0]); + } +} + } // namespace autofill |