Skip to content

GitLab

T
tangled
Commit 5b79ba3c authored by Hans Wennborg's avatar Hans Wennborg Committed by Commit Bot
Browse files
Options

Revert "Add HTTP-Bad Phase 3 experiment to field trial testing config" 

This reverts commit 1b6c18ab.

Reason for revert:
This broke two browser_tests in Official Windows builds:
SecurityStateTabHelperTest.DefaultSecurityLevelOnFilesystemUrl
SecurityStateTabHelperTest.DefaultSecurityLevelOnBlobUrl

Original change's description:
> Add HTTP-Bad Phase 3 experiment to field trial testing config
> 
> Some browser tests needed to be updated accordingly. I deleted a couple
> that were redundant with tests where the field trial is enabled. For a
> few others that only made sense in the pre-field trial world, I just
> disabled the feature for them, though we'll want to clean them up
> eventually after the field trial launches.
> 
> Bug: 807062
> Change-Id: I480b2527375368d6146c1ddace2551dece9ec596
> Reviewed-on: https://chromium-review.googlesource.com/891677
> Reviewed-by: Eric Lawrence <elawrence@chromium.org>
> Reviewed-by: Jesse Doherty <jwd@chromium.org>
> Commit-Queue: Emily Stark <estark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533845}

TBR=jwd@chromium.org,estark@chromium.org,elawrence@chromium.org

Change-Id: I34688c6ef005ac74e79d78a5b8d451790dba96a5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 807062, 808424
Reviewed-on: https://chromium-review.googlesource.com/899150Reviewed-by: default avatarHans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534048}
parent b8c96221
Hide whitespace changes
Inline Side-by-side
Showing with 134 additions and 117 deletions
chrome/browser/ssl/security_state_tab_helper_browsertest.cc
View file @ 5b79ba3c
......@@ -397,9 +397,11 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest {
}
// Navigates to an empty page and runs |javascript| to create a URL with with
// a scheme of |scheme|. Expects a security level of HTTP_SHOW_WARNING.
void TestBlobOrFilesystemURL(const std::string& scheme,
const std::string& javascript) {
// a scheme of |scheme|. If |expect_warning| is true, expects a password
// warning.
void TestPasswordFieldOnBlobOrFilesystemURL(const std::string& scheme,
const std::string& javascript,
bool expect_warning) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(contents);
......@@ -427,7 +429,15 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest {
contents->GetController().GetVisibleEntry();
ASSERT_TRUE(entry);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
security_state::InsecureInputEventData input_events = GetInputEvents(entry);
if (expect_warning) {
EXPECT_EQ(security_state::HTTP_SHOW_WARNING,
security_info.security_level);
EXPECT_TRUE(input_events.password_field_shown);
} else {
EXPECT_EQ(security_state::NONE, security_info.security_level);
EXPECT_FALSE(input_events.password_field_shown);
}
}
net::EmbeddedTestServer https_server_;
......@@ -1210,21 +1220,57 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
EXPECT_TRUE(GetInputEvents(entry).password_field_shown);
}
// Tests the default security level on blob URLs.
// Tests that when a visible password field is detected on a blob URL, the
// security level is downgraded to HTTP_SHOW_WARNING.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
PasswordSecurityLevelDowngradedOnBlobUrl) {
TestPasswordFieldOnBlobOrFilesystemURL(
"blob",
"var blob = new Blob(['<html><form><input type=password></form></html>'],"
" {type: 'text/html'});"
"window.domAutomationController.send(URL.createObjectURL(blob));",
true /* expect_warning */);
}
// Tests that when no password field is detected on a blob URL, the security
// level is not downgraded to HTTP_SHOW_WARNING.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
DefaultSecurityLevelOnBlobUrl) {
TestBlobOrFilesystemURL(
TestPasswordFieldOnBlobOrFilesystemURL(
"blob",
"var blob = new Blob(['<html>hello</html>'],"
"var blob = new Blob(['<html>no password or credit card field</html>'],"
" {type: 'text/html'});"
"window.domAutomationController.send(URL.createObjectURL(blob));");
"window.domAutomationController.send(URL.createObjectURL(blob));",
false /* expect_warning */);
}
// Same as PasswordSecurityLevelDowngradedOnBlobUrl, but instead of a blob URL,
// this creates a filesystem URL.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
PasswordSecurityLevelDowngradedOnFilesystemUrl) {
TestPasswordFieldOnBlobOrFilesystemURL(
"filesystem",
"window.webkitRequestFileSystem(window.TEMPORARY, 4096, function(fs) {"
" fs.root.getFile('test.html', {create: true}, function(fileEntry) {"
" fileEntry.createWriter(function(writer) {"
" writer.onwriteend = function(e) {"
" window.domAutomationController.send(fileEntry.toURL());"
" };"
" var blob ="
" new Blob(['<html><form><input type=password></form></html>'],"
" {type: 'text/html'});"
" writer.write(blob);"
" });"
" });"
"});",
true /* expect_warning */);
}
// Same as DefaultSecurityLevelOnBlobUrl, but instead of a blob URL,
// this creates a filesystem URL.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
DefaultSecurityLevelOnFilesystemUrl) {
TestBlobOrFilesystemURL(
TestPasswordFieldOnBlobOrFilesystemURL(
"filesystem",
"window.webkitRequestFileSystem(window.TEMPORARY, 4096, function(fs) {"
" fs.root.getFile('test.html', {create: true}, function(fileEntry) {"
......@@ -1233,22 +1279,19 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
" window.domAutomationController.send(fileEntry.toURL());"
" };"
" var blob ="
" new Blob(['<html>hello</html>'],"
" new Blob(['<html>no password or credit card field</html>'],"
" {type: 'text/html'});"
" writer.write(blob);"
" });"
" });"
"});");
"});",
false /* expect_warning */);
}
// Tests that when an invisible password field is present on an HTTP page load,
// the security level is *not* downgraded to HTTP_SHOW_WARNING.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
PasswordSecurityLevelNotDowngradedForInvisibleInput) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(contents);
......@@ -1360,17 +1403,83 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
EXPECT_FALSE(GetInputEvents(entry).password_field_shown);
}
// Tests that the security level of a HTTP page is downgraded to
// HTTP_SHOW_WARNING after editing a form field in the relevant configurations.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
SecurityLevelDowngradedAfterEditing) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
SecurityStateTabHelper* helper =
SecurityStateTabHelper::FromWebContents(contents);
ASSERT_TRUE(helper);
// Navigate to an HTTP page. Use a non-local hostname so that it is
// not considered secure.
ui_test_utils::NavigateToURL(
browser(),
GetURLWithNonLocalHostname(embedded_test_server(),
"/textinput/focus_input_on_load.html"));
security_state::SecurityInfo security_info;
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::NONE, security_info.security_level);
// Type one character into the focused input control and wait for a security
// state change.
SecurityStyleTestObserver observer(contents);
content::SimulateKeyPress(contents, ui::DomKey::FromCharacter('A'),
ui::DomCode::US_A, ui::VKEY_A, false, false, false,
false);
observer.WaitForDidChangeVisibleSecurityState();
// Verify that the security state degrades as expected.
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
EXPECT_TRUE(security_info.field_edit_downgraded_security_level);
EXPECT_EQ(1u, observer.latest_explanations().neutral_explanations.size());
content::NavigationEntry* entry = contents->GetController().GetVisibleEntry();
ASSERT_TRUE(entry);
EXPECT_TRUE(GetInputEvents(entry).insecure_field_edited);
{
// Ensure that the security level remains Dangerous in the
// kMarkHttpAsDangerous configuration.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
security_state::features::kMarkHttpAsFeature,
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterDangerous}});
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::DANGEROUS, security_info.security_level);
EXPECT_FALSE(security_info.field_edit_downgraded_security_level);
}
// Verify security state stays degraded after same-page navigation.
ui_test_utils::NavigateToURL(
browser(), GetURLWithNonLocalHostname(
embedded_test_server(),
"/textinput/focus_input_on_load.html#fragment"));
content::WaitForLoadStop(contents);
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
EXPECT_TRUE(security_info.field_edit_downgraded_security_level);
EXPECT_EQ(1u, observer.latest_explanations().neutral_explanations.size());
// Verify that after a refresh, the HTTP_SHOW_WARNING state is cleared.
contents->GetController().Reload(content::ReloadType::NORMAL, false);
content::WaitForLoadStop(contents);
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::NONE, security_info.security_level);
EXPECT_FALSE(security_info.field_edit_downgraded_security_level);
EXPECT_EQ(0u, observer.latest_explanations().neutral_explanations.size());
}
// Tests that the security level of a HTTP page is not downgraded when a form
// field is modified by JavaScript.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
SecurityLevelNotDowngradedAfterScriptModification) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
security_state::features::kMarkHttpAsFeature,
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::
kMarkHttpAsParameterWarningAndDangerousOnFormEdits}});
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
......@@ -1386,7 +1495,7 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
"/textinput/focus_input_on_load.html"));
security_state::SecurityInfo security_info;
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
EXPECT_EQ(security_state::NONE, security_info.security_level);
// Verify a value set operation isn't treated as user-input.
EXPECT_TRUE(content::ExecuteScript(
......@@ -1394,7 +1503,7 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
InjectScript(contents);
base::RunLoop().RunUntilIdle();
helper->GetSecurityInfo(&security_info);
ASSERT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
ASSERT_EQ(security_state::NONE, security_info.security_level);
ASSERT_FALSE(security_info.field_edit_downgraded_security_level);
// Verify an InsertText operation isn't treated as user-input.
......@@ -1403,7 +1512,7 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
InjectScript(contents);
base::RunLoop().RunUntilIdle();
helper->GetSecurityInfo(&security_info);
ASSERT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
ASSERT_EQ(security_state::NONE, security_info.security_level);
ASSERT_FALSE(security_info.field_edit_downgraded_security_level);
}
......@@ -1463,15 +1572,7 @@ void CheckForOneHttpWarningConsoleMessage(
// Tests that console messages are printed upon a call to
// GetSecurityInfo() on an HTTP_SHOW_WARNING page, exactly once per
// main-frame navigation.
//
// TODO(estark): add console messages for the |kMarkHttpAsParameterWarning|
// configuration of |kMarkHttpAsFeature| and update this test accordingly.
// https://crbug.com/802921
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, ConsoleMessage) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
ConsoleWebContentsDelegate* delegate = new ConsoleWebContentsDelegate(
Browser::CreateParams(browser()->profile(), true));
content::WebContents* original_contents =
......@@ -1538,16 +1639,8 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, ConsoleMessage) {
// Tests that additional HTTP_SHOW_WARNING console messages are not
// printed after subframe navigations.
//
// TODO(estark): add console messages for the |kMarkHttpAsParameterWarning|
// configuration of |kMarkHttpAsFeature| and update this test accordingly.
// https://crbug.com/802921
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
ConsoleMessageNotPrintedForFrameNavigation) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
ConsoleWebContentsDelegate* delegate = new ConsoleWebContentsDelegate(
Browser::CreateParams(browser()->profile(), true));
content::WebContents* original_contents =
......@@ -1629,16 +1722,8 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
// Tests that additional HTTP_SHOW_WARNING console messages are not
// printed after pushState navigations.
//
// TODO(estark): add console messages for the |kMarkHttpAsParameterWarning|
// configuration of |kMarkHttpAsFeature| and update this test accordingly.
// https://crbug.com/802921
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
ConsoleMessageNotPrintedForPushStateNavigation) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
ConsoleWebContentsDelegate* delegate = new ConsoleWebContentsDelegate(
Browser::CreateParams(browser()->profile(), true));
content::WebContents* original_contents =
......@@ -1894,10 +1979,6 @@ IN_PROC_BROWSER_TEST_F(DidChangeVisibleSecurityStateTest,
// to HTTP_SHOW_WARNING.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperIncognitoTest,
SecurityLevelDowngradedForHTTPInIncognito) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
ConsoleWebContentsDelegate* delegate = new ConsoleWebContentsDelegate(
Browser::CreateParams(browser()->profile(), true));
content::WebContents* original_contents =
......@@ -1949,16 +2030,8 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperIncognitoTest,
// Tests that additional HTTP_SHOW_WARNING console messages are not
// printed after aborted navigations.
//
// TODO(estark): add console messages for the |kMarkHttpAsParameterWarning|
// configuration of |kMarkHttpAsFeature| and update this test accordingly.
// https://crbug.com/802921
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperIncognitoTest,
ConsoleMessageNotPrintedForAbortedNavigation) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
ConsoleWebContentsDelegate* delegate = new ConsoleWebContentsDelegate(
Browser::CreateParams(browser()->profile(), true));
content::WebContents* original_contents =
......@@ -2025,10 +2098,6 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperIncognitoTest,
#endif
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
MAYBE_SecurityLevelNotDowngradedForHTTPInGuestMode) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
security_state::features::kMarkHttpAsFeature);
// Create a new browser in Guest Mode.
EXPECT_EQ(1U, BrowserList::GetInstance()->size());
content::WindowedNotificationObserver browser_creation_observer(
......@@ -2442,20 +2511,6 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
{
// Ensure that the security level remains Dangerous in the
// kMarkHttpAsDangerous configuration.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
security_state::features::kMarkHttpAsFeature,
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterDangerous}});
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::DANGEROUS, security_info.security_level);
EXPECT_FALSE(security_info.field_edit_downgraded_security_level);
}
// Type one character into the focused input control and wait for a security
// state change.
SecurityStyleTestObserver observer(contents);
......@@ -2467,21 +2522,6 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
// Verify that the security state degrades as expected.
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::DANGEROUS, security_info.security_level);
// Verify security state stays degraded after same-page navigation.
ui_test_utils::NavigateToURL(
browser(), GetURLWithNonLocalHostname(
embedded_test_server(),
"/textinput/focus_input_on_load.html#fragment"));
content::WaitForLoadStop(contents);
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::DANGEROUS, security_info.security_level);
// Verify that after a refresh, the DANGEROUS state is cleared.
contents->GetController().Reload(content::ReloadType::NORMAL, false);
content::WaitForLoadStop(contents);
helper->GetSecurityInfo(&security_info);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
}
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
......
testing/variations/fieldtrial_testing_config.json
View file @ 5b79ba3c
......@@ -1596,29 +1596,6 @@
]
}
],
"HTTPBadPhase3": [
{
"platforms": [
"android",
"chromeos",
"ios",
"linux",
"mac",
"win"
],
"experiments": [
{
"name": "NotSecureWarning",
"params": {
"treatment": "warning"
},
"enable_features": [
"MarkHttpAs"
]
}
]
}
],
"Html5ByDefault": [
{
"platforms": [
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment