Commit f28c4341 authored by yusukes's avatar yusukes Committed by Commit Bot

arc: Temporarily allow some of about/chrome URLs

to work around b/68953603.

BUG=b:68953603
TEST=try

Change-Id: I631493c78bf7ce69638661591203b02aa4402bf9
Reviewed-on: https://chromium-review.googlesource.com/766653
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: default avatarLuis Hector Chavez <lhchavez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516118}
parent 571fe62b
......@@ -18,7 +18,7 @@
#include "components/arc/audio/arc_audio_bridge.h"
#include "components/url_formatter/url_fixer.h"
#include "ui/base/layout.h"
#include "url/gurl.h"
#include "url/url_constants.h"
namespace arc {
namespace {
......@@ -61,6 +61,42 @@ class ArcIntentHelperBridgeFactory
// Base URL for the Chrome settings pages.
constexpr char kSettingsPageBaseUrl[] = "chrome://settings";
// TODO(yusukes): Properly fix b/68953603 and remove the constant.
constexpr const char* kWhitelistedUrls[] = {
"about:blank", "chrome://downloads", "chrome://history",
"chrome://settings",
};
// TODO(yusukes): Properly fix b/68953603 and remove the constant.
constexpr const char* kWhitelistedSettingsPaths[] = {
"accounts",
"appearance",
"autofill",
"bluetoothDevices",
"changePicture",
"clearBrowserData",
"cloudPrinters",
"cupsPrinters",
"dateTime",
"display",
"downloads",
"help",
"keyboard-overlay",
"languages",
"lockScreen",
"manageAccessibility",
"networks?type=VPN",
"onStartup",
"passwords",
"pointer-overlay",
"power",
"privacy",
"reset",
"search",
"storage",
"syncSetup",
};
} // namespace
// static
......@@ -128,7 +164,7 @@ void ArcIntentHelperBridge::OnOpenUrl(const std::string& url) {
// for example.
const GURL gurl(url_formatter::FixupURL(url, std::string()));
// Disallow opening chrome:// URLs.
if (!gurl.is_valid() || gurl.SchemeIs(kChromeUIScheme))
if (!gurl.is_valid() || !IsWhitelistedChromeUrl(gurl))
return;
open_url_delegate_->OpenUrl(gurl);
}
......@@ -265,4 +301,18 @@ void ArcIntentHelperBridge::SetOpenUrlDelegateForTesting(
open_url_delegate_ = std::move(open_url_delegate);
}
bool ArcIntentHelperBridge::IsWhitelistedChromeUrl(const GURL& url) {
if (!url.SchemeIs(kChromeUIScheme) && !url.SchemeIs(url::kAboutScheme))
return true;
if (whitelisted_urls_.empty()) {
whitelisted_urls_.insert(std::begin(kWhitelistedUrls),
std::end(kWhitelistedUrls));
const std::string prefix = "chrome://settings/";
for (const char* path : kWhitelistedSettingsPaths)
whitelisted_urls_.insert(GURL(prefix + path));
}
return whitelisted_urls_.count(url) > 0;
}
} // namespace arc
......@@ -6,6 +6,7 @@
#define COMPONENTS_ARC_INTENT_HELPER_ARC_INTENT_HELPER_BRIDGE_H_
#include <memory>
#include <set>
#include <string>
#include <vector>
......@@ -18,6 +19,7 @@
#include "components/arc/intent_helper/arc_intent_helper_observer.h"
#include "components/keyed_service/core/keyed_service.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "url/gurl.h"
class KeyedServiceBaseFactory;
......@@ -110,6 +112,11 @@ class ArcIntentHelperBridge
private:
THREAD_CHECKER(thread_checker_);
// Returns true if |url| is whitelisted. This function also returns true when
// |url| is neither chrome:// nor about:.
// TODO(yusukes): Properly fix b/68953603 and remove the function.
bool IsWhitelistedChromeUrl(const GURL& url);
content::BrowserContext* const context_;
ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager.
......@@ -123,6 +130,9 @@ class ArcIntentHelperBridge
base::ObserverList<ArcIntentHelperObserver> observer_list_;
// TODO(yusukes): Properly fix b/68953603 and remove the variable.
std::set<GURL> whitelisted_urls_;
DISALLOW_COPY_AND_ASSIGN(ArcIntentHelperBridge);
};
......
......@@ -290,14 +290,48 @@ TEST_F(ArcIntentHelperTest, TestOnOpenUrl_ChromeScheme) {
instance_->OnOpenUrl("chrome://www.google.com");
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("chrome://settings");
instance_->OnOpenUrl("chrome://help");
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("chrome://version");
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:");
instance_->OnOpenUrl("about:"); // this redirects to chrome://version
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
// Some of the about/chrome URLs are whitelisted (for now).
instance_->OnOpenUrl("about:blank");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:downloads");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:history");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/accounts");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/keyboard-overlay");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/networks?type=VPN");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/networks?type=this_is_not_whitelisted");
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/syncSetup");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("about:settings/thisIsNotWhitelisted");
EXPECT_FALSE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
instance_->OnOpenUrl("chrome://settings");
EXPECT_TRUE(test_open_url_delegate_->TakeLastOpenedUrl().is_valid());
}
// Tests that OnOpenChromeSettings opens the specified settings section in the
......
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