From 10bf1a7030eeed02e2af95eaff012d2428ea5bc2 Mon Sep 17 00:00:00 2001 From: "tommi@chromium.org" Date: Thu, 1 Oct 2009 16:00:21 +0000 Subject: Committing patch 256001 for Roger:http://codereview.chromium.org/256001Allow Chrome Frame to display chrome-extension URLs when running in privilegedmode.BUG=noneTEST=see unit tests Review URL: http://codereview.chromium.org/246050 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27724 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome_frame/bho.cc | 2 +- chrome_frame/chrome_active_document.cc | 4 +-- chrome_frame/chrome_frame_activex_base.h | 18 ++++++------- chrome_frame/chrome_frame_automation.cc | 8 +++--- chrome_frame/chrome_frame_automation.h | 2 +- chrome_frame/chrome_frame_npapi.cc | 6 ++--- chrome_frame/test/chrome_frame_automation_mock.h | 2 +- chrome_frame/test/chrome_frame_unittests.cc | 6 ++--- chrome_frame/test/util_unittests.cc | 33 ++++++++++++++++++++++++ chrome_frame/utils.cc | 5 +++- chrome_frame/utils.h | 3 ++- 11 files changed, 64 insertions(+), 25 deletions(-) (limited to 'chrome_frame') diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc index 9561cc1..52d19ba 100644 --- a/chrome_frame/bho.cc +++ b/chrome_frame/bho.cc @@ -88,7 +88,7 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url, std::wstring current_url; bool is_chrome_protocol = false; - if (is_top_level && IsValidUrlScheme(url->bstrVal)) { + if (is_top_level && IsValidUrlScheme(url->bstrVal, false)) { current_url.assign(url->bstrVal, SysStringLen(url->bstrVal)); is_chrome_protocol = StartsWith(current_url, kChromeProtocolPrefix, false); diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index 147ef0b..ad104c9 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -677,7 +677,7 @@ bool ChromeActiveDocument::ParseUrl(const std::wstring& url, !StartsWith(initial_url, kChromeAttachExternalTabPrefix, false); } - if (!IsValidUrlScheme(initial_url)) { + if (!IsValidUrlScheme(initial_url, is_privileged_)) { DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: " << url; return false; @@ -721,7 +721,7 @@ bool ChromeActiveDocument::LaunchUrl(const std::wstring& url, if (url_.Length()) { std::string utf8_url; WideToUTF8(url_, url_.Length(), &utf8_url); - if (!automation_client_->InitiateNavigation(utf8_url)) { + if (!automation_client_->InitiateNavigation(utf8_url, is_privileged_)) { DLOG(ERROR) << "Invalid URL: " << url; Error(L"Invalid URL"); url_.Reset(); diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h index bfdabb0..7754bc0 100644 --- a/chrome_frame/chrome_frame_activex_base.h +++ b/chrome_frame/chrome_frame_activex_base.h @@ -10,7 +10,7 @@ #include // Copied min/max defs from windows headers to appease atlimage.h. -// TODO(slightlyoff): Figure out of more recent platform SDK's (> 6.1) +// TODO(slightlyoff): Figure out of more recent platform SDK's (> 6.1) // undo the janky "#define NOMINMAX" train wreck. See: // http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=100703 #ifndef max @@ -282,13 +282,13 @@ END_MSG_MAP() return false; } - // Should connections initiated by this class try to block - // responses served with the X-Frame-Options header? - // ActiveX controls genereally will want to do this, - // returning true, while true top-level documents - // (ActiveDocument servers) will not. Your specialization - // of this template should implement this method based on how - // it "feels" from a security perspective. If it's hosted in another + // Should connections initiated by this class try to block + // responses served with the X-Frame-Options header? + // ActiveX controls genereally will want to do this, + // returning true, while true top-level documents + // (ActiveDocument servers) will not. Your specialization + // of this template should implement this method based on how + // it "feels" from a security perspective. If it's hosted in another // scriptable document, return true, else false. virtual bool is_frame_busting_enabled() const { return true; @@ -509,7 +509,7 @@ END_MSG_MAP() // We can initiate navigation here even if ready_state is not complete. // We do not have to set proxy, and AutomationClient will take care // of navigation just after CreateExternalTab is done. - if (!automation_client_->InitiateNavigation(full_url)) { + if (!automation_client_->InitiateNavigation(full_url, is_privileged_)) { // TODO(robertshield): Make InitiateNavigation return more useful // error information. return E_INVALIDARG; diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc index 1f369b5..a03a39c 100644 --- a/chrome_frame/chrome_frame_automation.cc +++ b/chrome_frame/chrome_frame_automation.cc @@ -477,15 +477,17 @@ void ChromeFrameAutomationClient::Uninitialize() { init_state_ = UNINITIALIZED; } -bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url) { +bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url, + bool is_privileged) { if (url.empty()) return false; url_ = GURL(url); // Catch invalid URLs early. - if (!url_.is_valid() || !IsValidUrlScheme(UTF8ToWide(url))) { - DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url; + if (!url_.is_valid() || !IsValidUrlScheme(UTF8ToWide(url), is_privileged)) { + DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url + << " is_privileged=" << is_privileged; return false; } diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h index 796facb..dfe408d 100644 --- a/chrome_frame/chrome_frame_automation.h +++ b/chrome_frame/chrome_frame_automation.h @@ -189,7 +189,7 @@ class ChromeFrameAutomationClient bool incognito); void Uninitialize(); - virtual bool InitiateNavigation(const std::string& url); + virtual bool InitiateNavigation(const std::string& url, bool is_privileged); virtual bool NavigateToIndex(int index); bool ForwardMessageFromExternalHost(const std::string& message, const std::string& origin, diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc index 0c58cff..f558fdd 100644 --- a/chrome_frame/chrome_frame_npapi.cc +++ b/chrome_frame/chrome_frame_npapi.cc @@ -409,7 +409,7 @@ bool ChromeFrameNPAPI::Initialize(NPMIMEType mime_type, NPP instance, // part of LaunchSettings /* if (!src_.empty()) - automation_client_->InitiateNavigation(src_); + automation_client_->InitiateNavigation(src_, is_privileged_); std::string proxy_settings; bool has_prefs = pref_service_->Initialize(instance_, @@ -1041,7 +1041,7 @@ void ChromeFrameNPAPI::OnAutomationServerReady() { } if (!src_.empty()) { - if (!automation_client_->InitiateNavigation(src_)) { + if (!automation_client_->InitiateNavigation(src_, is_privileged_)) { DLOG(ERROR) << "Failed to navigate to: " << src_; src_.clear(); } @@ -1309,7 +1309,7 @@ bool ChromeFrameNPAPI::NavigateToURL(const NPVariant* args, uint32_t arg_count, src_ = full_url; // Navigate only if we completed initialization i.e. proxy is set etc. if (ready_state_ == READYSTATE_COMPLETE) { - if (!automation_client_->InitiateNavigation(full_url)) { + if (!automation_client_->InitiateNavigation(full_url, is_privileged_)) { // TODO(tommi): call NPN_SetException. src_.clear(); return false; diff --git a/chrome_frame/test/chrome_frame_automation_mock.h b/chrome_frame/test/chrome_frame_automation_mock.h index 4c3fe7b..7678282 100644 --- a/chrome_frame/test/chrome_frame_automation_mock.h +++ b/chrome_frame/test/chrome_frame_automation_mock.h @@ -41,7 +41,7 @@ class AutomationMockDelegate // Navigate external tab to the specified url through automation bool Navigate(const std::string& url) { url_ = GURL(url); - return automation_client_->InitiateNavigation(url); + return automation_client_->InitiateNavigation(url, false); } // Navigate the external to a 'file://' url for unit test files diff --git a/chrome_frame/test/chrome_frame_unittests.cc b/chrome_frame/test/chrome_frame_unittests.cc index d24f5b3..206c6d6 100644 --- a/chrome_frame/test/chrome_frame_unittests.cc +++ b/chrome_frame/test/chrome_frame_unittests.cc @@ -1101,7 +1101,7 @@ TEST(CFACWithChrome, NavigateOk) { EXPECT_CALL(cfd, OnAutomationServerReady()) .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod( client.get(), &ChromeFrameAutomationClient::InitiateNavigation, - url)))); + url, false)))); // cfd.SetOnNavigationStateChanged(); EXPECT_CALL(cfd, @@ -1141,7 +1141,7 @@ TEST(CFACWithChrome, DISABLED_NavigateFailed) { EXPECT_CALL(cfd, OnAutomationServerReady()) .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod( client.get(), &ChromeFrameAutomationClient::InitiateNavigation, - url)))); + url, false)))); EXPECT_CALL(cfd, OnNavigationStateChanged(testing::_, testing::_)) @@ -1199,7 +1199,7 @@ TEST(CFACWithChrome, UseHostNetworkStack) { EXPECT_CALL(cfd, OnAutomationServerReady()) .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod( client.get(), &ChromeFrameAutomationClient::InitiateNavigation, - url)))); + url, false)))); EXPECT_CALL(cfd, OnNavigationStateChanged(testing::_, testing::_)) .Times(testing::AnyNumber()); diff --git a/chrome_frame/test/util_unittests.cc b/chrome_frame/test/util_unittests.cc index cc2727b..3f07b82 100644 --- a/chrome_frame/test/util_unittests.cc +++ b/chrome_frame/test/util_unittests.cc @@ -118,3 +118,36 @@ TEST(UtilTests, HaveSameOrigin) { EXPECT_EQ(test.same_origin, HaveSameOrigin(test.a, test.b)); } } + +TEST(UtilTests, IsValidUrlScheme) { + struct Cases { + const wchar_t* url; + bool is_privileged; + bool expected; + } test_cases[] = { + // non-privileged test cases + { L"http://www.google.ca", false, true }, + { L"https://www.google.ca", false, true }, + { L"about:config", false, true }, + { L"view-source:http://www.google.ca", false, true }, + { L"chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", false, false }, + { L"ftp://www.google.ca", false, false }, + { L"file://www.google.ca", false, false }, + { L"file://C:\boot.ini", false, false }, + + // privileged test cases + { L"http://www.google.ca", true, true }, + { L"https://www.google.ca", true, true }, + { L"about:config", true, true }, + { L"view-source:http://www.google.ca", true, true }, + { L"chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", true, true }, + { L"ftp://www.google.ca", true, false }, + { L"file://www.google.ca", true, false }, + { L"file://C:\boot.ini", true, false }, + }; + + for (int i = 0; i < arraysize(test_cases); ++i) { + const Cases& test = test_cases[i]; + EXPECT_EQ(test.expected, IsValidUrlScheme(test.url, test.is_privileged)); + } +} diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index b445c26..36e8afd 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -601,7 +601,7 @@ HRESULT GetUrlFromMoniker(IMoniker* moniker, IBindCtx* bind_context, return hr; } -bool IsValidUrlScheme(const std::wstring& url) { +bool IsValidUrlScheme(const std::wstring& url, bool is_privileged) { if (url.empty()) return false; @@ -611,6 +611,9 @@ bool IsValidUrlScheme(const std::wstring& url) { crack_url.SchemeIs("about") || crack_url.SchemeIs("view-source")) return true; + if (is_privileged && crack_url.SchemeIs("chrome-extension")) + return true; + if (StartsWith(url, kChromeAttachExternalTabPrefix, false)) return true; diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h index 80b9a53..baece31 100644 --- a/chrome_frame/utils.h +++ b/chrome_frame/utils.h @@ -219,7 +219,8 @@ HRESULT GetUrlFromMoniker(IMoniker* moniker, IBindCtx* bind_context, // Returns true if the URL passed in is something which can be handled by // Chrome. If this function returns false then we should fail the navigation. -bool IsValidUrlScheme(const std::wstring& url); +// When is_privileged is true, chrome extension URLs will be considered valid. +bool IsValidUrlScheme(const std::wstring& url, bool is_privileged); // This returns the base directory in which to store user profiles. bool GetUserProfileBaseDirectory(std::wstring* path); -- cgit v1.1