Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loading brave crash url doesn't cause renderer crash #2229

Merged
merged 2 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions browser/brave_scheme_load_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test_utils.h"
#include "net/dns/mock_host_resolver.h"
Expand Down Expand Up @@ -228,6 +230,18 @@ IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest,
"Not allowed to load local resource: brave://settings/"));
}

// Check renderer crash happened by observing related notification.
IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest, CrashURLTest) {
content::WindowedNotificationObserver observer(
content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
content::NotificationService::AllSources());
browser()->OpenURL(
content::OpenURLParams(GURL("brave://crash/"), content::Referrer(),
WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, false));
observer.Wait();
}

// Some webuis are not allowed to load in private window.
// Allowed url list are checked by IsURLAllowedInIncognito().
// So, corresponding brave scheme url should be filtered as chrome scheme.
Expand Down
17 changes: 11 additions & 6 deletions chromium_src/chrome/browser/ui/browser_navigator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@

#include "brave/browser/renderer_host/brave_navigation_ui_data.h"
#include "brave/common/webui_url_constants.h"
#include "chrome/browser/ui/browser_navigator_params.h"
#include "chrome/common/webui_url_constants.h"
#include "url/gurl.h"

namespace {
bool IsHostAllowedInIncognitoBraveImpl(std::string* scheme,
const base::StringPiece& host) {
if (*scheme == "brave")
*scheme = content::kChromeUIScheme;
void AdjustNavigateParamsForURLBraveImpl(NavigateParams* params) {
if (params->url.SchemeIs(content::kBraveUIScheme)) {
GURL::Replacements replacements;
replacements.SetSchemeStr(content::kChromeUIScheme);
params->url = params->url.ReplaceComponents(replacements);
}
}

bool IsHostAllowedInIncognitoBraveImpl(const base::StringPiece& host) {
if (host == kRewardsHost ||
host == kBraveUISyncHost ||
host == chrome::kChromeUISyncInternalsHost) {
Expand All @@ -22,8 +27,8 @@ bool IsHostAllowedInIncognitoBraveImpl(std::string* scheme,

return true;
}
}
} // namespace

#define ChromeNavigationUIData BraveNavigationUIData
#include "../../../../chrome/browser/ui/browser_navigator.cc"
#include "../../../../chrome/browser/ui/browser_navigator.cc" // NOLINT
#undef ChromeNavigationUIData
16 changes: 11 additions & 5 deletions patches/chrome-browser-ui-browser_navigator.cc.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index cd2ade8119331caa626270056a1863165afd3fb4..33d15bbf371940d35b73f2b8bca81bb6ebfef3ec 100644
index cd2ade8119331caa626270056a1863165afd3fb4..f3691ded29969615dd7225246467da8a62d0b8fe 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -723,6 +723,9 @@ void Navigate(NavigateParams* params) {
@@ -106,6 +106,7 @@ Browser* GetOrCreateBrowser(Profile* profile, bool user_gesture) {
// Returns true on success. Otherwise, if changing params leads the browser into
// an erroneous state, returns false.
bool AdjustNavigateParamsForURL(NavigateParams* params) {
+ AdjustNavigateParamsForURLBraveImpl(params);
if (params->contents_to_insert || params->switch_to_singleton_tab ||
IsURLAllowedInIncognito(params->url, params->initiating_profile) ||
params->initiating_profile->IsGuestSession()) {
@@ -723,6 +724,7 @@ void Navigate(NavigateParams* params) {
bool IsHostAllowedInIncognito(const GURL& url) {
std::string scheme = url.scheme();
base::StringPiece host = url.host_piece();
+#if defined(BRAVE_CHROMIUM_BUILD)
+ if (!IsHostAllowedInIncognitoBraveImpl(&scheme, host)) return false;
+#endif
+ if (!IsHostAllowedInIncognitoBraveImpl(host)) return false;
if (scheme == chrome::kChromeSearchScheme) {
return host != chrome::kChromeUIThumbnailHost &&
host != chrome::kChromeUIThumbnailHost2 &&
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 90cfe806ba0bef2182f7a1cc7ad02c0010dc2136..86a096b70e3df14d247da8a37359411cbb373c1c 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -2765,16 +2765,17 @@ void NavigationControllerImpl::NavigateWithoutEntry(
// Note: we intentionally leave the pending entry in place for renderer debug
// URLs, unlike the cases below where we clear it if the navigation doesn't
// proceed.
- if (IsRendererDebugURL(params.url)) {
+ const GURL url = params.url.SchemeIs(url::kJavaScriptScheme) ? params.url : pending_entry_->GetURL();
+ if (IsRendererDebugURL(url)) {
// Renderer-debug URLs won't go through NavigationThrottlers so we have to
// check them explicitly. See bug 913334.
if (GetContentClient()->browser()->IsRendererDebugURLBlacklisted(
- params.url, browser_context_)) {
+ url, browser_context_)) {
DiscardPendingEntry(false);
return;
}

- HandleRendererDebugURL(node, params.url);
+ HandleRendererDebugURL(node, url);
return;
}