Commit 1bb21e33 authored by Wei-Yin Chen (陳威尹)'s avatar Wei-Yin Chen (陳威尹) Committed by Commit Bot

Validate the URL parameter in DOM distiller viewer

In chrome-distiller:// URLs, validate the URL parameter by adding
its hash as part of the host. This way, changing the URL parameter
requires changing the origin as well, so it's harder to be misused.

TBR=sky@chromium.org

Bug: 991888
Change-Id: Ida641f697d77a2c9c1a93266208a55c0e773aeaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763628
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarDavid Benjamin <davidben@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693009}
parent d8cf0175
...@@ -165,6 +165,7 @@ class DomDistillerViewerSourceBrowserTest : public InProcessBrowserTest { ...@@ -165,6 +165,7 @@ class DomDistillerViewerSourceBrowserTest : public InProcessBrowserTest {
void ViewSingleDistilledPage(const GURL& url, void ViewSingleDistilledPage(const GURL& url,
const std::string& expected_mime_type); const std::string& expected_mime_type);
void ViewSingleDistilledPageAndExpectErrorPage(const GURL& url);
void PrefTest(bool is_error_page); void PrefTest(bool is_error_page);
// Database entries. // Database entries.
static FakeDB<ArticleEntry>::EntryMap* database_model_; static FakeDB<ArticleEntry>::EntryMap* database_model_;
...@@ -219,10 +220,10 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, ...@@ -219,10 +220,10 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
// We should expect distillation for any valid URL. // We should expect distillation for any valid URL.
expect_distillation_ = true; expect_distillation_ = true;
expect_distiller_page_ = true; expect_distiller_page_ = true;
GURL view_url("http://www.example.com/1"); GURL original_url("http://www.example.com/1");
const GURL url = const GURL view_url =
url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme, view_url); url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme, original_url);
ViewSingleDistilledPage(url, "text/html"); ViewSingleDistilledPage(view_url, "text/html");
} }
void DomDistillerViewerSourceBrowserTest::ViewSingleDistilledPage( void DomDistillerViewerSourceBrowserTest::ViewSingleDistilledPage(
...@@ -256,9 +257,13 @@ void DomDistillerViewerSourceBrowserTest::ViewSingleDistilledPage( ...@@ -256,9 +257,13 @@ void DomDistillerViewerSourceBrowserTest::ViewSingleDistilledPage(
IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
MAYBE_TestBadUrlErrorPage) { MAYBE_TestBadUrlErrorPage) {
GURL url("chrome-distiller://bad"); GURL url("chrome-distiller://bad");
ViewSingleDistilledPageAndExpectErrorPage(url);
}
void DomDistillerViewerSourceBrowserTest::
ViewSingleDistilledPageAndExpectErrorPage(const GURL& url) {
// Navigate to a distiller URL. // Navigate to a distiller URL.
ui_test_utils::NavigateToURL(browser(), url); ViewSingleDistilledPage(url, "text/html");
content::WebContents* contents = content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
...@@ -319,6 +324,19 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, ...@@ -319,6 +324,19 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
ViewSingleDistilledPage(url, "text/html"); ViewSingleDistilledPage(url, "text/html");
} }
IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
InvalidURLShouldGetErrorPage) {
const GURL original_url("http://www.example.com/1");
const GURL different_url("http://www.example.com/2");
const GURL view_url =
url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme, original_url);
// This is a bogus URL, so no distillation will happen.
const GURL bad_view_url = net::AppendOrReplaceQueryParameter(
view_url, kUrlKey, different_url.spec());
ViewSingleDistilledPageAndExpectErrorPage(bad_view_url);
}
IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, EarlyTemplateLoad) { IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, EarlyTemplateLoad) {
dom_distiller::DomDistillerServiceFactory::GetInstance() dom_distiller::DomDistillerServiceFactory::GetInstance()
->SetTestingFactoryAndUse(browser()->profile(), ->SetTestingFactoryAndUse(browser()->profile(),
...@@ -553,20 +571,21 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, ...@@ -553,20 +571,21 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
} }
void DomDistillerViewerSourceBrowserTest::PrefTest(bool is_error_page) { void DomDistillerViewerSourceBrowserTest::PrefTest(bool is_error_page) {
GURL url; GURL view_url;
if (is_error_page) { if (is_error_page) {
expect_distillation_ = false; expect_distillation_ = false;
expect_distiller_page_ = false; expect_distiller_page_ = false;
url = GURL("chrome-distiller://bad"); view_url = GURL("chrome-distiller://bad");
} else { } else {
expect_distillation_ = true; expect_distillation_ = true;
expect_distiller_page_ = true; expect_distiller_page_ = true;
GURL view_url("http://www.example.com/1"); GURL original_url("http://www.example.com/1");
url = url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme, view_url); view_url = url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme,
original_url);
} }
content::WebContents* contents = content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
ViewSingleDistilledPage(url, "text/html"); ViewSingleDistilledPage(view_url, "text/html");
content::WaitForLoadStop(contents); content::WaitForLoadStop(contents);
ExpectBodyHasThemeAndFont(contents, "light", "sans-serif"); ExpectBodyHasThemeAndFont(contents, "light", "sans-serif");
......
...@@ -8,6 +8,7 @@ include_rules = [ ...@@ -8,6 +8,7 @@ include_rules = [
"+components/sync/protocol", "+components/sync/protocol",
"+components/sync_preferences", "+components/sync_preferences",
"+components/variations", "+components/variations",
"+crypto", # For sha256
"+google", # For third_party/protobuf. "+google", # For third_party/protobuf.
"+third_party/dom_distiller_js", "+third_party/dom_distiller_js",
"+third_party/re2", "+third_party/re2",
......
...@@ -55,8 +55,7 @@ class DomDistillerViewerSource::RequestViewerHandle ...@@ -55,8 +55,7 @@ class DomDistillerViewerSource::RequestViewerHandle
public content::WebContentsObserver { public content::WebContentsObserver {
public: public:
RequestViewerHandle(content::WebContents* web_contents, RequestViewerHandle(content::WebContents* web_contents,
const std::string& expected_scheme, const GURL& expected_url,
const std::string& expected_request_path,
DistilledPagePrefs* distilled_page_prefs, DistilledPagePrefs* distilled_page_prefs,
DistillerUIHandle* ui_handle); DistillerUIHandle* ui_handle);
~RequestViewerHandle() override; ~RequestViewerHandle() override;
...@@ -83,11 +82,8 @@ class DomDistillerViewerSource::RequestViewerHandle ...@@ -83,11 +82,8 @@ class DomDistillerViewerSource::RequestViewerHandle
// cancelled. // cancelled.
void Cancel(); void Cancel();
// The scheme hosting the current view request; // The URL hosting the current view request;
std::string expected_scheme_; const GURL expected_url_;
// The query path for the current view request.
std::string expected_request_path_;
// Whether the page is sufficiently initialized to handle updates from the // Whether the page is sufficiently initialized to handle updates from the
// distiller. // distiller.
...@@ -108,13 +104,11 @@ class DomDistillerViewerSource::RequestViewerHandle ...@@ -108,13 +104,11 @@ class DomDistillerViewerSource::RequestViewerHandle
DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle( DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
content::WebContents* web_contents, content::WebContents* web_contents,
const std::string& expected_scheme, const GURL& expected_url,
const std::string& expected_request_path,
DistilledPagePrefs* distilled_page_prefs, DistilledPagePrefs* distilled_page_prefs,
DistillerUIHandle* ui_handle) DistillerUIHandle* ui_handle)
: DomDistillerRequestViewBase(distilled_page_prefs), : DomDistillerRequestViewBase(distilled_page_prefs),
expected_scheme_(expected_scheme), expected_url_(expected_url),
expected_request_path_(expected_request_path),
waiting_for_page_ready_(true), waiting_for_page_ready_(true),
distiller_ui_handle_(ui_handle) { distiller_ui_handle_(ui_handle) {
content::WebContentsObserver::Observe(web_contents); content::WebContentsObserver::Observe(web_contents);
...@@ -146,9 +140,7 @@ void DomDistillerViewerSource::RequestViewerHandle::DidFinishNavigation( ...@@ -146,9 +140,7 @@ void DomDistillerViewerSource::RequestViewerHandle::DidFinishNavigation(
return; return;
const GURL& navigation = navigation_handle->GetURL(); const GURL& navigation = navigation_handle->GetURL();
bool expected_main_view_request = bool expected_main_view_request = navigation == expected_url_;
navigation.SchemeIs(expected_scheme_) &&
expected_request_path_ == navigation.query();
if (navigation_handle->IsSameDocument() || expected_main_view_request) { if (navigation_handle->IsSameDocument() || expected_main_view_request) {
// In-page navigations, as well as the main view request can be ignored. // In-page navigations, as well as the main view request can be ignored.
if (expected_main_view_request) { if (expected_main_view_request) {
...@@ -260,21 +252,29 @@ void DomDistillerViewerSource::StartDataRequest( ...@@ -260,21 +252,29 @@ void DomDistillerViewerSource::StartDataRequest(
} }
} }
// An empty |path| is invalid, but guard against it. If not empty, assume // We need the host part to validate the parameter, but it's not available
// |path| starts with '?', which is stripped away. // from |URLDataSource|. |web_contents| is the most convenient place to
const std::string path_after_query_separator = // obtain the full URL.
path.size() > 0 ? path.substr(1) : ""; // TODO(crbug.com/991888): pass GURL in URLDataSource::StartDataRequest().
const std::string query = GURL("https://host/" + path).query();
GURL request_url = web_contents->GetVisibleURL();
// The query should match what's seen in |web_contents|.
// For javascript:window.open(), it's not the case, but it's not a supported
// use case.
if (request_url.query() != query || request_url.path() != "/") {
request_url = GURL();
}
RequestViewerHandle* request_viewer_handle = RequestViewerHandle* request_viewer_handle =
new RequestViewerHandle(web_contents, scheme_, path_after_query_separator, new RequestViewerHandle(web_contents, request_url,
dom_distiller_service_->GetDistilledPagePrefs(), dom_distiller_service_->GetDistilledPagePrefs(),
distiller_ui_handle_.get()); distiller_ui_handle_.get());
std::unique_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest( std::unique_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest(
dom_distiller_service_, path, request_viewer_handle, dom_distiller_service_, request_url, request_viewer_handle,
web_contents->GetContainerBounds().size()); web_contents->GetContainerBounds().size());
GURL current_url(url_utils::GetValueForKeyInUrlPathQuery(path, kUrlKey)); GURL current_url(url_utils::GetOriginalUrlFromDistillerUrl(request_url));
std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml( std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml(
url_utils::GetOriginalUrlFromDistillerUrl(current_url).spec(), current_url.spec(),
dom_distiller_service_->GetDistilledPagePrefs()->GetTheme(), dom_distiller_service_->GetDistilledPagePrefs()->GetTheme(),
dom_distiller_service_->GetDistilledPagePrefs()->GetFontFamily()); dom_distiller_service_->GetDistilledPagePrefs()->GetFontFamily());
......
...@@ -8,11 +8,14 @@ ...@@ -8,11 +8,14 @@
#include "base/guid.h" #include "base/guid.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "components/dom_distiller/core/url_constants.h" #include "components/dom_distiller/core/url_constants.h"
#include "components/grit/components_resources.h" #include "components/grit/components_resources.h"
#include "crypto/sha2.h"
#include "net/base/url_util.h" #include "net/base/url_util.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/url_util.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -21,6 +24,12 @@ namespace url_utils { ...@@ -21,6 +24,12 @@ namespace url_utils {
namespace { namespace {
const char kDummyInternalUrlPrefix[] = "chrome-distiller-internal://dummy/"; const char kDummyInternalUrlPrefix[] = "chrome-distiller-internal://dummy/";
const char kSeparator[] = "_";
std::string SHA256InHex(base::StringPiece str) {
std::string sha256 = crypto::SHA256HashString(str);
return base::ToLowerASCII(base::HexEncode(sha256.c_str(), sha256.size()));
}
} // namespace } // namespace
...@@ -31,28 +40,44 @@ const GURL GetDistillerViewUrlFromEntryId(const std::string& scheme, ...@@ -31,28 +40,44 @@ const GURL GetDistillerViewUrlFromEntryId(const std::string& scheme,
} }
const GURL GetDistillerViewUrlFromUrl(const std::string& scheme, const GURL GetDistillerViewUrlFromUrl(const std::string& scheme,
const GURL& view_url, const GURL& url,
int64_t start_time_ms) { int64_t start_time_ms) {
GURL url(scheme + "://" + base::GenerateGUID()); GURL view_url(scheme + "://" + base::GenerateGUID() + kSeparator +
SHA256InHex(url.spec()));
if (start_time_ms > 0) { if (start_time_ms > 0) {
url = net::AppendOrReplaceQueryParameter( view_url = net::AppendOrReplaceQueryParameter(
url, kTimeKey, base::NumberToString(start_time_ms)); view_url, kTimeKey, base::NumberToString(start_time_ms));
} }
return net::AppendOrReplaceQueryParameter(url, kUrlKey, view_url.spec()); return net::AppendOrReplaceQueryParameter(view_url, kUrlKey, url.spec());
} }
const GURL GetOriginalUrlFromDistillerUrl(const GURL& url) { const GURL GetOriginalUrlFromDistillerUrl(const GURL& url) {
if (!dom_distiller::url_utils::IsDistilledPage(url)) if (!IsDistilledPage(url))
return url; return url;
std::string original_url_str; std::string original_url_str;
net::GetValueForKeyInQuery(url, kUrlKey, &original_url_str); net::GetValueForKeyInQuery(url, kUrlKey, &original_url_str);
return GURL(original_url_str); // Make sure kDomDistillerScheme is considered standard scheme for
// |GURL::host_piece()| to work correctly.
DCHECK(url::IsStandard(kDomDistillerScheme,
url::Component(0, strlen(kDomDistillerScheme))));
std::vector<base::StringPiece> pieces =
base::SplitStringPiece(url.host_piece(), kSeparator,
base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
if (pieces.size() != 2)
return GURL();
if (SHA256InHex(original_url_str) != pieces[1])
return GURL();
const GURL original_url(original_url_str);
if (!IsUrlDistillable(original_url))
return GURL();
return original_url;
} }
int64_t GetTimeFromDistillerUrl(const GURL& url) { int64_t GetTimeFromDistillerUrl(const GURL& url) {
if (!dom_distiller::url_utils::IsDistilledPage(url)) if (!IsDistilledPage(url))
return 0; return 0;
std::string time_str; std::string time_str;
......
...@@ -25,7 +25,9 @@ const GURL GetDistillerViewUrlFromUrl(const std::string& scheme, ...@@ -25,7 +25,9 @@ const GURL GetDistillerViewUrlFromUrl(const std::string& scheme,
int64_t start_time_ms = 0); int64_t start_time_ms = 0);
// Returns the original URL from the distilled URL. // Returns the original URL from the distilled URL.
// If the URL is not distilled, it is returned as is. // If |distilled_url| is not distilled, it is returned as is.
// If |distilled_url| looks like distilled, but no original URL can be found,
// an empty, invalid URL is returned.
const GURL GetOriginalUrlFromDistillerUrl(const GURL& distilled_url); const GURL GetOriginalUrlFromDistillerUrl(const GURL& distilled_url);
// Returns the starting time from the distilled URL. // Returns the starting time from the distilled URL.
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#include "components/dom_distiller/core/url_utils.h" #include "components/dom_distiller/core/url_utils.h"
#include "components/dom_distiller/core/url_constants.h" #include "components/dom_distiller/core/url_constants.h"
#include "net/base/url_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/url_util.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -15,9 +17,12 @@ namespace url_utils { ...@@ -15,9 +17,12 @@ namespace url_utils {
TEST(DomDistillerUrlUtilsTest, TestPathUtil) { TEST(DomDistillerUrlUtilsTest, TestPathUtil) {
const std::string single_key = "mypath?foo=bar"; const std::string single_key = "mypath?foo=bar";
EXPECT_EQ("bar", GetValueForKeyInUrlPathQuery(single_key, "foo")); EXPECT_EQ("bar", GetValueForKeyInUrlPathQuery(single_key, "foo"));
const std::string two_keys = "mypath?key1=foo&key2=bar"; const std::string two_keys = "mypath?key1=foo&key2=bar";
EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(two_keys, "key1")); EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(two_keys, "key1"));
EXPECT_EQ("bar", GetValueForKeyInUrlPathQuery(two_keys, "key2")); EXPECT_EQ("bar", GetValueForKeyInUrlPathQuery(two_keys, "key2"));
// First occurrence wins.
const std::string multiple_same_key = "mypath?key=foo&key=bar"; const std::string multiple_same_key = "mypath?key=foo&key=bar";
EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(multiple_same_key, "key")); EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(multiple_same_key, "key"));
} }
...@@ -41,16 +46,43 @@ TEST(DomDistillerUrlUtilsTest, TestGetValueForKeyInUrlPathQuery) { ...@@ -41,16 +46,43 @@ TEST(DomDistillerUrlUtilsTest, TestGetValueForKeyInUrlPathQuery) {
EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(valid_url_two_keys, "key")); EXPECT_EQ("foo", GetValueForKeyInUrlPathQuery(valid_url_two_keys, "key"));
} }
std::string ThroughDistiller(const std::string& url) { void AssertEqualExceptHost(const GURL& a, const GURL& b) {
return GetOriginalUrlFromDistillerUrl( url::Replacements<char> no_host;
GetDistillerViewUrlFromUrl(kDomDistillerScheme, GURL(url), 123)) no_host.ClearHost();
.spec(); EXPECT_EQ(a.ReplaceComponents(no_host), b.ReplaceComponents(no_host));
}
TEST(DomDistillerUrlUtilsTest, TestGetDistillerViewUrlFromUrl) {
AssertEqualExceptHost(
GURL("chrome-distiller://any/"
"?time=123&url=http%3A%2F%2Fexample.com%2Fpath%3Fq%3Dabc%26p%3D1%"
"23anchor"),
GetDistillerViewUrlFromUrl(
kDomDistillerScheme, GURL("http://example.com/path?q=abc&p=1#anchor"),
123));
} }
std::string GetOriginalUrlFromDistillerUrl(const std::string& url) { std::string GetOriginalUrlFromDistillerUrl(const std::string& url) {
return GetOriginalUrlFromDistillerUrl(GURL(url)).spec(); return GetOriginalUrlFromDistillerUrl(GURL(url)).spec();
} }
TEST(DomDistillerUrlUtilsTest, TestGetOriginalUrlFromDistillerUrl) {
EXPECT_EQ(
"http://example.com/path?q=abc&p=1#anchor",
GetOriginalUrlFromDistillerUrl(
"chrome-distiller://"
"any_"
"d091ebf8f841eae9ca23822c3d0f369c16d3748478d0b74111be176eb96722e5/"
"?time=123&url=http%3A%2F%2Fexample.com%2Fpath%3Fq%3Dabc%26p%3D1%"
"23anchor"));
}
std::string ThroughDistiller(const std::string& url) {
return GetOriginalUrlFromDistillerUrl(
GetDistillerViewUrlFromUrl(kDomDistillerScheme, GURL(url), 123))
.spec();
}
TEST(DomDistillerUrlUtilsTest, TestDistillerEndToEnd) { TEST(DomDistillerUrlUtilsTest, TestDistillerEndToEnd) {
// Tests a normal url. // Tests a normal url.
const std::string url = "http://example.com/"; const std::string url = "http://example.com/";
...@@ -65,8 +97,24 @@ TEST(DomDistillerUrlUtilsTest, TestDistillerEndToEnd) { ...@@ -65,8 +97,24 @@ TEST(DomDistillerUrlUtilsTest, TestDistillerEndToEnd) {
// Tests a url with file:// scheme. // Tests a url with file:// scheme.
const std::string url_file = "file:///home/userid/path/index.html"; const std::string url_file = "file:///home/userid/path/index.html";
EXPECT_EQ(url_file, ThroughDistiller(url_file)); EXPECT_EQ("", ThroughDistiller(url_file));
EXPECT_EQ(url_file, GetOriginalUrlFromDistillerUrl(url_file)); EXPECT_EQ(url_file, GetOriginalUrlFromDistillerUrl(url_file));
// Tests a nested url.
const std::string nested_url =
GetDistillerViewUrlFromUrl(kDomDistillerScheme, GURL(url)).spec();
EXPECT_EQ("", ThroughDistiller(nested_url));
EXPECT_EQ(url, GetOriginalUrlFromDistillerUrl(nested_url));
}
TEST(DomDistillerUrlUtilsTest, TestRejectInvalidURLs) {
const std::string url = "http://example.com/";
const std::string url2 = "http://example.org/";
const GURL view_url =
GetDistillerViewUrlFromUrl(kDomDistillerScheme, GURL(url), 123);
GURL bad_view_url =
net::AppendOrReplaceQueryParameter(view_url, kUrlKey, url2);
EXPECT_EQ(GURL(), GetOriginalUrlFromDistillerUrl(bad_view_url));
} }
} // namespace url_utils } // namespace url_utils
......
...@@ -250,17 +250,17 @@ const std::string GetJavaScript() { ...@@ -250,17 +250,17 @@ const std::string GetJavaScript() {
std::unique_ptr<ViewerHandle> CreateViewRequest( std::unique_ptr<ViewerHandle> CreateViewRequest(
DomDistillerServiceInterface* dom_distiller_service, DomDistillerServiceInterface* dom_distiller_service,
const std::string& path, const GURL& url,
ViewRequestDelegate* view_request_delegate, ViewRequestDelegate* view_request_delegate,
const gfx::Size& render_view_size) { const gfx::Size& render_view_size) {
std::string entry_id = if (!url_utils::IsDistilledPage(url)) {
url_utils::GetValueForKeyInUrlPathQuery(path, kEntryIdKey); return nullptr;
}
std::string entry_id = url_utils::GetValueForKeyInUrl(url, kEntryIdKey);
bool has_valid_entry_id = !entry_id.empty(); bool has_valid_entry_id = !entry_id.empty();
entry_id = base::ToUpperASCII(entry_id); entry_id = base::ToUpperASCII(entry_id);
std::string requested_url_str = GURL requested_url(url_utils::GetOriginalUrlFromDistillerUrl(url));
url_utils::GetValueForKeyInUrlPathQuery(path, kUrlKey);
GURL requested_url(requested_url_str);
bool has_valid_url = url_utils::IsUrlDistillable(requested_url); bool has_valid_url = url_utils::IsUrlDistillable(requested_url);
if (has_valid_entry_id && has_valid_url) { if (has_valid_entry_id && has_valid_url) {
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/distilled_page_prefs.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "url/gurl.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -75,7 +76,7 @@ const std::string GetJavaScript(); ...@@ -75,7 +76,7 @@ const std::string GetJavaScript();
// viewing distilled content based on the |path|. // viewing distilled content based on the |path|.
std::unique_ptr<ViewerHandle> CreateViewRequest( std::unique_ptr<ViewerHandle> CreateViewRequest(
DomDistillerServiceInterface* dom_distiller_service, DomDistillerServiceInterface* dom_distiller_service,
const std::string& path, const GURL& url,
ViewRequestDelegate* view_request_delegate, ViewRequestDelegate* view_request_delegate,
const gfx::Size& render_view_size); const gfx::Size& render_view_size);
......
...@@ -9,11 +9,24 @@ ...@@ -9,11 +9,24 @@
#include "components/dom_distiller/core/dom_distiller_test_util.h" #include "components/dom_distiller/core/dom_distiller_test_util.h"
#include "components/dom_distiller/core/task_tracker.h" #include "components/dom_distiller/core/task_tracker.h"
#include "components/dom_distiller/core/url_constants.h" #include "components/dom_distiller/core/url_constants.h"
#include "components/dom_distiller/core/url_utils.h"
#include "net/base/url_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/url_util.h"
namespace dom_distiller { namespace dom_distiller {
const char kTestScheme[] = "myscheme"; namespace {
const GURL GetDistillerViewUrlFromUrl(const std::string& url) {
return url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme, GURL(url));
}
const GURL GetDistillerViewUrlFromEntryId(const std::string& id) {
return url_utils::GetDistillerViewUrlFromEntryId(kDomDistillerScheme, id);
}
} // namespace
class FakeViewRequestDelegate : public ViewRequestDelegate { class FakeViewRequestDelegate : public ViewRequestDelegate {
public: public:
...@@ -78,10 +91,10 @@ class DomDistillerViewerTest : public testing::Test { ...@@ -78,10 +91,10 @@ class DomDistillerViewerTest : public testing::Test {
protected: protected:
std::unique_ptr<ViewerHandle> CreateViewRequest( std::unique_ptr<ViewerHandle> CreateViewRequest(
const std::string& path, const GURL& url,
ViewRequestDelegate* view_request_delegate) { ViewRequestDelegate* view_request_delegate) {
return viewer::CreateViewRequest(service_.get(), path, return viewer::CreateViewRequest(service_.get(), url, view_request_delegate,
view_request_delegate, gfx::Size()); gfx::Size());
} }
std::unique_ptr<TestDomDistillerService> service_; std::unique_ptr<TestDomDistillerService> service_;
...@@ -94,9 +107,8 @@ TEST_F(DomDistillerViewerTest, TestCreatingViewUrlRequest) { ...@@ -94,9 +107,8 @@ TEST_F(DomDistillerViewerTest, TestCreatingViewUrlRequest) {
EXPECT_CALL(*service_, ViewUrlImpl()) EXPECT_CALL(*service_, ViewUrlImpl())
.WillOnce(testing::Return(viewer_handle)); .WillOnce(testing::Return(viewer_handle));
EXPECT_CALL(*service_, ViewEntryImpl()).Times(0); EXPECT_CALL(*service_, ViewEntryImpl()).Times(0);
CreateViewRequest( CreateViewRequest(GetDistillerViewUrlFromUrl("http://www.example.com/"),
std::string("?") + kUrlKey + "=http%3A%2F%2Fwww.example.com%2F", view_request_delegate.get());
view_request_delegate.get());
} }
TEST_F(DomDistillerViewerTest, TestCreatingViewEntryRequest) { TEST_F(DomDistillerViewerTest, TestCreatingViewEntryRequest) {
...@@ -106,7 +118,7 @@ TEST_F(DomDistillerViewerTest, TestCreatingViewEntryRequest) { ...@@ -106,7 +118,7 @@ TEST_F(DomDistillerViewerTest, TestCreatingViewEntryRequest) {
EXPECT_CALL(*service_, ViewEntryImpl()) EXPECT_CALL(*service_, ViewEntryImpl())
.WillOnce(testing::Return(viewer_handle)); .WillOnce(testing::Return(viewer_handle));
EXPECT_CALL(*service_, ViewUrlImpl()).Times(0); EXPECT_CALL(*service_, ViewUrlImpl()).Times(0);
CreateViewRequest(std::string("?") + kEntryIdKey + "=abc-def", CreateViewRequest(GetDistillerViewUrlFromEntryId("abc-def"),
view_request_delegate.get()); view_request_delegate.get());
} }
...@@ -116,19 +128,24 @@ TEST_F(DomDistillerViewerTest, TestCreatingInvalidViewRequest) { ...@@ -116,19 +128,24 @@ TEST_F(DomDistillerViewerTest, TestCreatingInvalidViewRequest) {
EXPECT_CALL(*service_, ViewEntryImpl()).Times(0); EXPECT_CALL(*service_, ViewEntryImpl()).Times(0);
EXPECT_CALL(*service_, ViewUrlImpl()).Times(0); EXPECT_CALL(*service_, ViewUrlImpl()).Times(0);
// Specify none of the required query parameters. // Specify none of the required query parameters.
CreateViewRequest("?foo=bar", view_request_delegate.get()); CreateViewRequest(GURL(std::string(kDomDistillerScheme) + "://host?foo=bar"),
view_request_delegate.get());
// Specify both of the required query parameters. // Specify both of the required query parameters.
CreateViewRequest("?" + std::string(kUrlKey) + CreateViewRequest(net::AppendOrReplaceQueryParameter(
"=http%3A%2F%2Fwww.example.com%2F&" + GetDistillerViewUrlFromUrl("http://www.example.com/"),
std::string(kEntryIdKey) + "=abc-def", kEntryIdKey, "abc-def"),
view_request_delegate.get()); view_request_delegate.get());
// Specify an internal Chrome page. // Specify an internal Chrome page.
CreateViewRequest("?" + std::string(kUrlKey) + "=chrome%3A%2F%2Fsettings%2F", CreateViewRequest(GetDistillerViewUrlFromUrl("chrome://settings/"),
view_request_delegate.get()); view_request_delegate.get());
// Specify a recursive URL. // Specify a recursive URL.
CreateViewRequest("?" + std::string(kUrlKey) + "=" + CreateViewRequest(GetDistillerViewUrlFromUrl(
std::string(kTestScheme) + "%3A%2F%2Fabc-def%2F", GetDistillerViewUrlFromEntryId("abc-def").spec()),
view_request_delegate.get()); view_request_delegate.get());
// Specify a non-distilled URL.
CreateViewRequest(GURL("https://example.com"), view_request_delegate.get());
// Specify an empty URL.
CreateViewRequest(GURL(), view_request_delegate.get());
} }
DistilledPagePrefs* TestDomDistillerService::GetDistilledPagePrefs() { DistilledPagePrefs* TestDomDistillerService::GetDistilledPagePrefs() {
......
...@@ -90,6 +90,7 @@ class ComponentsTestSuite : public base::TestSuite { ...@@ -90,6 +90,7 @@ class ComponentsTestSuite : public base::TestSuite {
url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST); url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
url::AddStandardScheme("devtools", url::SCHEME_WITH_HOST); url::AddStandardScheme("devtools", url::SCHEME_WITH_HOST);
url::AddStandardScheme("chrome-search", url::SCHEME_WITH_HOST); url::AddStandardScheme("chrome-search", url::SCHEME_WITH_HOST);
url::AddStandardScheme("chrome-distiller", url::SCHEME_WITH_HOST);
ContentSettingsPattern::SetNonWildcardDomainNonPortSchemes( ContentSettingsPattern::SetNonWildcardDomainNonPortSchemes(
kNonWildcardDomainNonPortSchemes, kNonWildcardDomainNonPortSchemes,
......
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