From c4b8fd74e67dfb23ef150c6aa313cb506746f06f Mon Sep 17 00:00:00 2001 From: meacer Date: Mon, 23 Feb 2015 17:30:08 -0800 Subject: [PATCH] Only accept HTTP and HTTPS urls for opensearch descriptor. This CL prevents http: or https: urls from referring to file: urls as search descriptor xmls. Allowing urls to refer to other urls with the same scheme has been considered (e.g. a file: url referring to an OSDD xml from a file: url), but this currently doesn't work for file: urls because of http://b/863583, so is not implemented here. BUG=429838 Review URL: https://codereview.chromium.org/917313004 Cr-Commit-Position: refs/heads/master@{#317723} --- .../ui/search_engines/search_engine_tab_helper.cc | 6 +- .../search_engine_tab_helper_browsertest.cc | 114 +++++++++++++++++++++ chrome/chrome_tests.gypi | 1 + 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 chrome/browser/ui/search_engines/search_engine_tab_helper_browsertest.cc diff --git a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc index 71bb7abca477..c1763ef840b8 100644 --- a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc +++ b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc @@ -110,8 +110,12 @@ void SearchEngineTabHelper::OnPageHasOSDD( // keyword. // Make sure that the page is the current page and other basic checks. - if (!osdd_url.is_valid()) + // When |page_url| has file: scheme, this method doesn't work because of + // http://b/issue?id=863583. For that reason, this doesn't check and allow + // urls referring to osdd urls with same schemes. + if (!osdd_url.is_valid() || !osdd_url.SchemeIsHTTPOrHTTPS()) return; + Profile* profile = Profile::FromBrowserContext(web_contents()->GetBrowserContext()); if (page_url != web_contents()->GetLastCommittedURL() || diff --git a/chrome/browser/ui/search_engines/search_engine_tab_helper_browsertest.cc b/chrome/browser/ui/search_engines/search_engine_tab_helper_browsertest.cc new file mode 100644 index 000000000000..80e80329f4f2 --- /dev/null +++ b/chrome/browser/ui/search_engines/search_engine_tab_helper_browsertest.cc @@ -0,0 +1,114 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/strings/stringprintf.h" +#include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/search_engines/search_engine_tab_helper.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "components/search_engines/template_url.h" +#include "components/search_engines/template_url_service.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/embedded_test_server/http_request.h" +#include "net/test/embedded_test_server/http_response.h" + +using net::test_server::BasicHttpResponse; +using net::test_server::EmbeddedTestServer; +using net::test_server::HttpRequest; +using net::test_server::HttpResponse; + +namespace { + +class TemplateURLServiceObserver { + public: + TemplateURLServiceObserver(TemplateURLService* service, base::RunLoop* loop) + : runner_(loop) { + DCHECK(loop); + template_url_sub_ = service->RegisterOnLoadedCallback(base::Bind( + &TemplateURLServiceObserver::StopLoop, base::Unretained(this))); + service->Load(); + } + ~TemplateURLServiceObserver() {} + + private: + void StopLoop() { runner_->Quit(); } + base::RunLoop* runner_; + scoped_ptr template_url_sub_; + + DISALLOW_COPY_AND_ASSIGN(TemplateURLServiceObserver); +}; + +testing::AssertionResult VerifyTemplateURLServiceLoad( + TemplateURLService* service) { + if (service->loaded()) + return testing::AssertionSuccess(); + base::RunLoop runner; + TemplateURLServiceObserver observer(service, &runner); + runner.Run(); + if (service->loaded()) + return testing::AssertionSuccess(); + return testing::AssertionFailure() << "TemplateURLService isn't loaded"; +} + +} // namespace + +class SearchEngineTabHelperBrowserTest : public InProcessBrowserTest { + public: + SearchEngineTabHelperBrowserTest() {} + ~SearchEngineTabHelperBrowserTest() override {} + + private: + scoped_ptr HandleRequest(const GURL& osdd_xml_url, + const HttpRequest& request) { + std::string html = base::StringPrintf( + "" + "" + " " + "" + "", + osdd_xml_url.spec().c_str()); + + scoped_ptr http_response(new BasicHttpResponse()); + http_response->set_code(net::HTTP_OK); + http_response->set_content(html); + http_response->set_content_type("text/html"); + return http_response.Pass(); + } + + // Starts a test server that serves a page pointing to a opensearch descriptor + // from a file:// url. + bool StartTestServer() { + GURL file_url = ui_test_utils::GetTestUrl( + base::FilePath(), + base::FilePath().AppendASCII("simple_open_search.xml")); + embedded_test_server()->RegisterRequestHandler( + base::Bind(&SearchEngineTabHelperBrowserTest::HandleRequest, + base::Unretained(this), file_url)); + return embedded_test_server()->InitializeAndWaitUntilReady(); + } + + void SetUpOnMainThread() override { ASSERT_TRUE(StartTestServer()); } + + DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelperBrowserTest); +}; + +IN_PROC_BROWSER_TEST_F(SearchEngineTabHelperBrowserTest, + IgnoreSearchDescriptionsFromFileURLs) { + TemplateURLService* url_service = + TemplateURLServiceFactory::GetForProfile(browser()->profile()); + ASSERT_TRUE(url_service); + VerifyTemplateURLServiceLoad(url_service); + TemplateURLService::TemplateURLVector template_urls = + url_service->GetTemplateURLs(); + // Navigate to a page with a search descriptor. Path doesn't matter as the + // test server always serves the same HTML. + GURL url(embedded_test_server()->GetURL("/")); + ui_test_utils::NavigateToURL(browser(), url); + // No new search engines should be added. + EXPECT_EQ(template_urls, url_service->GetTemplateURLs()); +} diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 1ef71c72b11d..82fcec020e3b 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -482,6 +482,7 @@ 'browser/ui/pdf/pdf_browsertest.cc', 'browser/ui/prefs/prefs_tab_helper_browsertest.cc', 'browser/ui/search/new_tab_page_interceptor_browsertest.cc', + 'browser/ui/search_engines/search_engine_tab_helper_browsertest.cc', 'browser/ui/settings_window_manager_browsertest.cc', 'browser/ui/startup/startup_browser_creator_browsertest.cc', 'browser/ui/sync/one_click_signin_bubble_links_delegate_browsertest.cc', -- 2.11.4.GIT