Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(802)

Unified Diff: content/browser/browser_plugin/browser_plugin_guest.cc

Issue 140073002: <webview>: navigating to WebStore should fire a loadabort instead of crashing. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Added test Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/browser_plugin/browser_plugin_guest.cc
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 03f7b1891730043302363195e6cc1192af57b167..797f66d066f430870e6895acae047a2ccfb12298 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -397,7 +397,32 @@ void BrowserPluginGuest::LoadURLWithParams(const GURL& url,
const Referrer& referrer,
PageTransition transition_type,
WebContents* web_contents) {
- NavigationController::LoadURLParams load_url_params(url);
+ // Do not allow navigating a guest to schemes other than known safe schemes.
+ // This will block the embedder trying to load unwanted schemes, e.g.
+ // chrome://settings.
+ bool scheme_is_blocked =
+ (!ChildProcessSecurityPolicyImpl::GetInstance()->IsWebSafeScheme(
+ url.scheme()) &&
+ !ChildProcessSecurityPolicyImpl::GetInstance()->IsPseudoScheme(
+ url.scheme())) ||
+ url.SchemeIs(kJavaScriptScheme);
+ bool can_commit =
+ GetContentClient()->browser()->CanCommitURL(
+ GetWebContents()->GetRenderProcessHost(), url);
+ if (scheme_is_blocked || !url.is_valid() || !can_commit) {
+ if (delegate_) {
+ std::string error_type;
+ base::RemoveChars(net::ErrorToString(net::ERR_ABORTED), "net::",
+ &error_type);
+ delegate_->LoadAbort(true /* is_top_level */, url, error_type);
+ }
+ return;
+ }
+
+ GURL validated_url(url);
+ GetWebContents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
+
+ NavigationController::LoadURLParams load_url_params(validated_url);
load_url_params.referrer = referrer;
load_url_params.transition_type = transition_type;
load_url_params.extra_headers = std::string();
@@ -586,7 +611,7 @@ void BrowserPluginGuest::Initialize(
renderer_prefs->report_frame_name_changes = true;
// Navigation is disabled in Chrome Apps. We want to make sure guest-initiated
// navigations still continue to function inside the app.
- renderer_prefs->browser_handles_all_top_level_requests = false;
+ renderer_prefs->browser_handles_all_top_level_requests = true;
lazyboy 2014/01/16 22:31:24 I remember creis@ raising concerns doing this befo
Fady Samuel 2014/01/16 23:40:27 Nasko? Thoughts? Charlie is on paternity leave.
// Disable "client blocked" error page for browser plugin.
renderer_prefs->disable_client_blocked_error_page = true;
@@ -813,7 +838,7 @@ WebContents* BrowserPluginGuest::OpenURLFromTab(WebContents* source,
return NULL;
}
if (params.disposition == CURRENT_TAB) {
- // This can happen for cross-site redirects.
+ // This can happen for cross-site redirects and top-level frame navigations.
LoadURLWithParams(params.url, params.referrer, params.transition, source);
return source;
}
@@ -1431,32 +1456,11 @@ void BrowserPluginGuest::OnNavigateGuest(
const std::string& src) {
GURL url = delegate_ ? delegate_->ResolveURL(src) : GURL(src);
- // Do not allow navigating a guest to schemes other than known safe schemes.
- // This will block the embedder trying to load unwanted schemes, e.g.
- // chrome://settings.
- bool scheme_is_blocked =
- (!ChildProcessSecurityPolicyImpl::GetInstance()->IsWebSafeScheme(
- url.scheme()) &&
- !ChildProcessSecurityPolicyImpl::GetInstance()->IsPseudoScheme(
- url.scheme())) ||
- url.SchemeIs(kJavaScriptScheme);
- if (scheme_is_blocked || !url.is_valid()) {
- if (delegate_) {
- std::string error_type;
- base::RemoveChars(net::ErrorToString(net::ERR_ABORTED), "net::",
- &error_type);
- delegate_->LoadAbort(true /* is_top_level */, url, error_type);
- }
- return;
- }
-
- GURL validated_url(url);
- GetWebContents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
// As guests do not swap processes on navigation, only navigations to
// normal web URLs are supported. No protocol handlers are installed for
// other schemes (e.g., WebUI or extensions), and no permissions or bindings
// can be granted to the guest process.
- LoadURLWithParams(validated_url, Referrer(), PAGE_TRANSITION_AUTO_TOPLEVEL,
+ LoadURLWithParams(url, Referrer(), PAGE_TRANSITION_AUTO_TOPLEVEL,
GetWebContents());
}

Powered by Google App Engine
This is Rietveld 408576698