Commit 3ede2886 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

DumpWithoutCrashing for navigations to about:version and similar URLs.

Navigations to about:version should either be blocked by
RenderProcessHostImpl::FilterURL or rewritten to navigations to
chrome://version/ by url_formatter::FixupURL.  This CL tries to help
catch cases that violate this assumption, by adding an early
NOTREACHED and DumpWithoutCrashing calls.

Bug: 1145717
Change-Id: Icbae9530ca851ae55d44947ac2f8efb754beec90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519861
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825012}
parent 55661826
......@@ -316,7 +316,7 @@ TEST_F(SiteDataRecorderTest, FeatureEventsGetForwardedWhenInBackground) {
EXPECT_CALL(*mock_writer, NotifySiteUnloaded(TabVisibility::kBackground));
}));
NavigatePageNodeOnUIThread(web_contents(), GURL("about://blank"));
NavigatePageNodeOnUIThread(web_contents(), GURL("about:blank"));
}
TEST_F(SiteDataRecorderTest, FeatureEventsIgnoredWhenLoadingInBackground) {
......
......@@ -266,10 +266,10 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, OnlyHttpContentsThrottled) {
}
{
// Expect about:// contents not to be throttled.
// Expect about:blank contents not to be throttled.
SCOPED_TRACE("about");
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("about://blank"));
web_contents(), GURL("about:blank"));
EXPECT_FALSE(policy()->ShouldThrottleWebContents(web_contents()));
ExpectThrottledPageCount(0);
}
......
......@@ -1040,6 +1040,31 @@ NavigationRequest::NavigationRequest(
DCHECK(browser_initiated_ || common_params_->initiator_origin.has_value());
DCHECK(!IsRendererDebugURL(common_params_->url));
DCHECK(common_params_->method == "POST" || !common_params_->post_data);
ScopedNavigationRequestCrashKeys crash_keys(this);
// There should be no navigations to about:newtab, about:version or other
// similar URLs (see https://crbug.com/1145717):
//
// 1. For URLs coming from outside the browser (e.g. from user input into the
// omnibox, from other apps, etc) the //content embedder should fix
// the URL using the url_formatter::FixupURL API from
// //components/url_formatter (which would for example translate
// "about:version" into "chrome://version/", "localhost:1234" into
// "http://localhost:1234/", etc.).
//
// 2. Most tests should directly use correct, final URLs (e.g.
// chrome://version instead of about:version; or about:blank instead of
// about://blank). Similarly, links in the product (e.g. links inside
// chrome://about/) should use correct, final URLs.
//
// 3. Renderer-initiated navigations (e.g. ones initiated via
// <a href="...">...</a> links embedded in web pages) should typically be
// blocked (via RenderProcessHostImpl::FilterURL).
if (GetURL().SchemeIs(url::kAboutScheme) && !GetURL().IsAboutBlank() &&
!GetURL().IsAboutSrcdoc()) {
NOTREACHED();
base::debug::DumpWithoutCrashing();
}
TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("navigation", "NavigationRequest",
navigation_id_, "navigation_request",
......
......@@ -21,7 +21,7 @@ import org.chromium.content_public.browser.test.util.TestThreadUtils;
*/
@RunWith(BaseJUnit4ClassRunner.class)
public class DialogOverlayImplTest {
private static final String BLANK_URL = "about://blank";
private static final String BLANK_URL = "about:blank";
@Rule
public DialogOverlayImplTestRule mActivityTestRule =
......
......@@ -45,7 +45,7 @@ public class WebContentsTest {
@Rule
public ContentShellActivityTestRule mActivityTestRule = new ContentShellActivityTestRule();
private static final String TEST_URL_1 = "about://blank";
private static final String TEST_URL_1 = "about:blank";
private static final String TEST_URL_2 = UrlUtils.encodeHtmlDataUri("<html>1</html>");
private static final String WEB_CONTENTS_KEY = "WEBCONTENTSKEY";
private static final String PARCEL_STRING_TEST_DATA = "abcdefghijklmnopqrstuvwxyz";
......
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