Skip to content

Commit

Permalink
Merge pull request #2229 from brave/fix_brave_crash_url
Browse files Browse the repository at this point in the history
Fix loading brave crash url doesn't cause renderer crash
  • Loading branch information
simonhong committed Apr 24, 2019
2 parents 5aacdbc + f24b048 commit eaba810
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 11 deletions.
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;
}

0 comments on commit eaba810

Please sign in to comment.