Skip to content

GitLab

T
tangled
Commit 0aa57604 authored by Charlie Reis's avatar Charlie Reis Committed by Commit Bot
Browse files
Options

Don't preserve NavigationEntry for failed navigations with invalid URLs. 

The formatting logic may rewrite such URLs into an unsafe state.  This
is a first step before preventing navigations to invalid URLs entirely.

Bug: 850824
Change-Id: I71743bfb4b610d55ce901ee8902125f934a2bb23
Reviewed-on: https://chromium-review.googlesource.com/c/1252942Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597304}
parent d5d142b6
Hide whitespace changes
Inline Side-by-side
Showing with 50 additions and 2 deletions
chrome/browser/chrome_navigation_browsertest.cc
View file @ 0aa57604
...@@ -449,6 +449,32 @@ IN_PROC_BROWSER_TEST_F(ChromeNavigationPortMappedBrowserTest, ...@@ -449,6 +449,32 @@ IN_PROC_BROWSER_TEST_F(ChromeNavigationPortMappedBrowserTest,
EXPECT_EQ(GURL(), new_web_contents->GetLastCommittedURL()); EXPECT_EQ(GURL(), new_web_contents->GetLastCommittedURL());
} }
// Ensure that a failed navigation in a new tab will not leave an invalid
// visible URL, which may be formatted in an unsafe way in the omnibox.
// See https://crbug.com/850824.
IN_PROC_BROWSER_TEST_F(ChromeNavigationBrowserTest,
ClearInvalidPendingURLOnFail) {
GURL initial_url = embedded_test_server()->GetURL(
"/frame_tree/invalid_link_to_new_window.html");
// Navigate to a page with a link that opens an invalid URL in a new window.
ui_test_utils::NavigateToURL(browser(), initial_url);
content::WebContents* main_contents =
browser()->tab_strip_model()->GetActiveWebContents();
// Simulate a click on the link and wait for the new window.
content::WebContentsAddedObserver new_tab_observer;
EXPECT_TRUE(ExecuteScript(main_contents, "simulateClick()"));
content::WebContents* new_contents = new_tab_observer.GetWebContents();
// The load in the new window should fail.
EXPECT_FALSE(WaitForLoadStop(new_contents));
// Ensure that there is no pending entry or visible URL.
EXPECT_EQ(nullptr, new_contents->GetController().GetPendingEntry());
EXPECT_EQ(GURL(), new_contents->GetVisibleURL());
}
// A test performing two simultaneous navigations, to ensure code in chrome/, // A test performing two simultaneous navigations, to ensure code in chrome/,
// such as tab helpers, can handle those cases. // such as tab helpers, can handle those cases.
// This test starts a browser-initiated cross-process navigation, which is // This test starts a browser-initiated cross-process navigation, which is
......
chrome/test/data/frame_tree/invalid_link_to_new_window.html 0 → 100644
View file @ 0aa57604
<!doctype html>
<html>
<head>
<script>
function simulateClick() {
var evt = new MouseEvent("click", {});
target = document.getElementById("invalid_url_link");
return target.dispatchEvent(evt);
}
</script>
</head>
<body>
<a href="a.a:@javascript:foo()"
id="invalid_url_link"
target="win">Link to invalid URL in new window</a>
</body>
</html>
content/browser/frame_host/navigator_impl.cc
View file @ 0aa57604
...@@ -765,9 +765,14 @@ void NavigatorImpl::DiscardPendingEntryIfNeeded(int expected_pending_entry_id) { ...@@ -765,9 +765,14 @@ void NavigatorImpl::DiscardPendingEntryIfNeeded(int expected_pending_entry_id) {
// allow the view to clear the pending entry and typed URL if the user // allow the view to clear the pending entry and typed URL if the user
// requests (e.g., hitting Escape with focus in the address bar). // requests (e.g., hitting Escape with focus in the address bar).
// //
// Do not leave the pending entry visible if it has an invalid URL, since this
// might be formatted in an unexpected or unsafe way.
// TODO(creis): Block navigations to invalid URLs in https://crbug.com/850824.
//
// Note: don't touch the transient entry, since an interstitial may exist. // Note: don't touch the transient entry, since an interstitial may exist.
bool should_preserve_entry = controller_->IsUnmodifiedBlankTab() || bool should_preserve_entry = pending_entry->GetURL().is_valid() &&
delegate_->ShouldPreserveAbortedURLs(); (controller_->IsUnmodifiedBlankTab() ||
delegate_->ShouldPreserveAbortedURLs());
if (pending_entry != controller_->GetVisibleEntry() || if (pending_entry != controller_->GetVisibleEntry() ||
!should_preserve_entry) { !should_preserve_entry) {
controller_->DiscardPendingEntry(true); controller_->DiscardPendingEntry(true);
......
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