Commit e677556d authored by Dominick Ng's avatar Dominick Ng Committed by Commit Bot

Add request_initiator to the WebApkIconHasher's icon fetch.

The WebAPK icon hasher makes a no-CORS fetch request for the primary and
badge icons for creating WebAPKs. Before this CL, the request was made
without setting a request_initiator on the fetch request. When the
requested image is protected by Cross-Origin-Resource-Policy: same-site,
the lack of initiator causes the fetch to fail and WebAPK creation fails
as a result.

This CL fixed the issue by setting the initiator to the origin of the
requested start URL for the WebAPK. A test is added to ensure correct
behaviour.

BUG=977829

Change-Id: I308a21667b42e9f4c46806deda261fbebfeb46a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1673671
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarPeter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671934}
parent 17341ca2
...@@ -38,15 +38,18 @@ std::string ComputeMurmur2Hash(const std::string& raw_image_data) { ...@@ -38,15 +38,18 @@ std::string ComputeMurmur2Hash(const std::string& raw_image_data) {
// static // static
void WebApkIconHasher::DownloadAndComputeMurmur2Hash( void WebApkIconHasher::DownloadAndComputeMurmur2Hash(
network::mojom::URLLoaderFactory* url_loader_factory, network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
const Murmur2HashCallback& callback) { const Murmur2HashCallback& callback) {
DownloadAndComputeMurmur2HashWithTimeout( DownloadAndComputeMurmur2HashWithTimeout(
url_loader_factory, icon_url, kDownloadTimeoutInMilliseconds, callback); url_loader_factory, request_initiator, icon_url,
kDownloadTimeoutInMilliseconds, callback);
} }
// static // static
void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout( void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
network::mojom::URLLoaderFactory* url_loader_factory, network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
int timeout_ms, int timeout_ms,
const Murmur2HashCallback& callback) { const Murmur2HashCallback& callback) {
...@@ -69,11 +72,13 @@ void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout( ...@@ -69,11 +72,13 @@ void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
} }
// The icon hasher will delete itself when it is done. // The icon hasher will delete itself when it is done.
new WebApkIconHasher(url_loader_factory, icon_url, timeout_ms, callback); new WebApkIconHasher(url_loader_factory, request_initiator, icon_url,
timeout_ms, callback);
} }
WebApkIconHasher::WebApkIconHasher( WebApkIconHasher::WebApkIconHasher(
network::mojom::URLLoaderFactory* url_loader_factory, network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
int timeout_ms, int timeout_ms,
const Murmur2HashCallback& callback) const Murmur2HashCallback& callback)
...@@ -86,6 +91,7 @@ WebApkIconHasher::WebApkIconHasher( ...@@ -86,6 +91,7 @@ WebApkIconHasher::WebApkIconHasher(
base::Unretained(this))); base::Unretained(this)));
auto resource_request = std::make_unique<network::ResourceRequest>(); auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->request_initiator = request_initiator;
resource_request->url = icon_url; resource_request->url = icon_url;
simple_url_loader_ = network::SimpleURLLoader::Create( simple_url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), std::move(resource_request),
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h"
namespace network { namespace network {
class SimpleURLLoader; class SimpleURLLoader;
...@@ -33,17 +34,20 @@ class WebApkIconHasher { ...@@ -33,17 +34,20 @@ class WebApkIconHasher {
// the image cannot not be downloaded in time (e.g. 404 HTTP error code). // the image cannot not be downloaded in time (e.g. 404 HTTP error code).
static void DownloadAndComputeMurmur2Hash( static void DownloadAndComputeMurmur2Hash(
network::mojom::URLLoaderFactory* url_loader_factory, network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
const Murmur2HashCallback& callback); const Murmur2HashCallback& callback);
static void DownloadAndComputeMurmur2HashWithTimeout( static void DownloadAndComputeMurmur2HashWithTimeout(
network::mojom::URLLoaderFactory* url_loader_factory, network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
int timeout_ms, int timeout_ms,
const Murmur2HashCallback& callback); const Murmur2HashCallback& callback);
private: private:
WebApkIconHasher(network::mojom::URLLoaderFactory* url_loader_factory, WebApkIconHasher(network::mojom::URLLoaderFactory* url_loader_factory,
const url::Origin& request_initiator,
const GURL& icon_url, const GURL& icon_url,
int timeout_ms, int timeout_ms,
const Murmur2HashCallback& callback); const Murmur2HashCallback& callback);
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "services/network/test/test_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.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/origin.h"
namespace { namespace {
...@@ -38,7 +39,7 @@ class WebApkIconHasherRunner { ...@@ -38,7 +39,7 @@ class WebApkIconHasherRunner {
void Run(network::mojom::URLLoaderFactory* url_loader_factory, void Run(network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url) { const GURL& icon_url) {
WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout( WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
url_loader_factory, icon_url, 300, url_loader_factory, url::Origin::Create(icon_url), icon_url, 300,
base::Bind(&WebApkIconHasherRunner::OnCompleted, base::Bind(&WebApkIconHasherRunner::OnCompleted,
base::Unretained(this))); base::Unretained(this)));
......
...@@ -53,6 +53,7 @@ ...@@ -53,6 +53,7 @@
#include "ui/gfx/android/java_bitmap.h" #include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/codec/png_codec.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h"
namespace { namespace {
...@@ -611,6 +612,7 @@ void WebApkInstaller::OnHaveSufficientSpaceForInstall() { ...@@ -611,6 +612,7 @@ void WebApkInstaller::OnHaveSufficientSpaceForInstall() {
// should be fast because the icon should be in the HTTP cache. // should be fast because the icon should be in the HTTP cache.
WebApkIconHasher::DownloadAndComputeMurmur2Hash( WebApkIconHasher::DownloadAndComputeMurmur2Hash(
GetURLLoaderFactory(browser_context_), GetURLLoaderFactory(browser_context_),
url::Origin::Create(install_shortcut_info_->url),
install_shortcut_info_->best_primary_icon_url, install_shortcut_info_->best_primary_icon_url,
base::Bind(&WebApkInstaller::OnGotPrimaryIconMurmur2Hash, base::Bind(&WebApkInstaller::OnGotPrimaryIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
...@@ -629,6 +631,7 @@ void WebApkInstaller::OnGotPrimaryIconMurmur2Hash( ...@@ -629,6 +631,7 @@ void WebApkInstaller::OnGotPrimaryIconMurmur2Hash(
install_shortcut_info_->best_primary_icon_url) { install_shortcut_info_->best_primary_icon_url) {
WebApkIconHasher::DownloadAndComputeMurmur2Hash( WebApkIconHasher::DownloadAndComputeMurmur2Hash(
GetURLLoaderFactory(browser_context_), GetURLLoaderFactory(browser_context_),
url::Origin::Create(install_shortcut_info_->url),
install_shortcut_info_->best_badge_icon_url, install_shortcut_info_->best_badge_icon_url,
base::Bind(&WebApkInstaller::OnGotBadgeIconMurmur2Hash, base::Bind(&WebApkInstaller::OnGotBadgeIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr(), true, primary_icon_hash)); weak_ptr_factory_.GetWeakPtr(), true, primary_icon_hash));
......
...@@ -41,12 +41,18 @@ const base::FilePath::CharType kTestDataDir[] = ...@@ -41,12 +41,18 @@ const base::FilePath::CharType kTestDataDir[] =
// URL of mock WebAPK server. // URL of mock WebAPK server.
const char* kServerUrl = "/webapkserver/"; const char* kServerUrl = "/webapkserver/";
// Start URL for the WebAPK
const char* kStartUrl = "/index.html";
// The URLs of best icons from Web Manifest. We use a random file in the test // The URLs of best icons from Web Manifest. We use a random file in the test
// data directory. Since WebApkInstaller does not try to decode the file as an // data directory. Since WebApkInstaller does not try to decode the file as an
// image it is OK that the file is not an image. // image it is OK that the file is not an image.
const char* kBestPrimaryIconUrl = "/simple.html"; const char* kBestPrimaryIconUrl = "/simple.html";
const char* kBestBadgeIconUrl = "/nostore.html"; const char* kBestBadgeIconUrl = "/nostore.html";
// Icon which has Cross-Origin-Resource-Policy: same-origin set.
const char* kBestPrimaryIconCorpUrl = "/banners/image-512px-corp.png";
// Token from the WebAPK server. In production, the token is sent to Google // Token from the WebAPK server. In production, the token is sent to Google
// Play. Google Play uses the token to retrieve the WebAPK from the WebAPK // Play. Google Play uses the token to retrieve the WebAPK from the WebAPK
// server. // server.
...@@ -95,10 +101,12 @@ class TestWebApkInstaller : public WebApkInstaller { ...@@ -95,10 +101,12 @@ class TestWebApkInstaller : public WebApkInstaller {
class WebApkInstallerRunner { class WebApkInstallerRunner {
public: public:
WebApkInstallerRunner(content::BrowserContext* browser_context, WebApkInstallerRunner(content::BrowserContext* browser_context,
const GURL& start_url,
const GURL& best_primary_icon_url, const GURL& best_primary_icon_url,
const GURL& best_badge_icon_url, const GURL& best_badge_icon_url,
SpaceStatus test_space_status) SpaceStatus test_space_status)
: browser_context_(browser_context), : browser_context_(browser_context),
start_url_(start_url),
best_primary_icon_url_(best_primary_icon_url), best_primary_icon_url_(best_primary_icon_url),
best_badge_icon_url_(best_badge_icon_url), best_badge_icon_url_(best_badge_icon_url),
test_space_status_(test_space_status) {} test_space_status_(test_space_status) {}
...@@ -109,7 +117,7 @@ class WebApkInstallerRunner { ...@@ -109,7 +117,7 @@ class WebApkInstallerRunner {
base::RunLoop run_loop; base::RunLoop run_loop;
on_completed_callback_ = run_loop.QuitClosure(); on_completed_callback_ = run_loop.QuitClosure();
ShortcutInfo info((GURL())); ShortcutInfo info(start_url_);
info.best_primary_icon_url = best_primary_icon_url_; info.best_primary_icon_url = best_primary_icon_url_;
info.best_badge_icon_url = best_badge_icon_url_; info.best_badge_icon_url = best_badge_icon_url_;
WebApkInstaller::InstallAsyncForTesting( WebApkInstaller::InstallAsyncForTesting(
...@@ -152,6 +160,8 @@ class WebApkInstallerRunner { ...@@ -152,6 +160,8 @@ class WebApkInstallerRunner {
content::BrowserContext* browser_context_; content::BrowserContext* browser_context_;
const GURL start_url_;
// The Web Manifest's icon URLs. // The Web Manifest's icon URLs.
const GURL best_primary_icon_url_; const GURL best_primary_icon_url_;
const GURL best_badge_icon_url_; const GURL best_badge_icon_url_;
...@@ -302,6 +312,9 @@ class WebApkInstallerTest : public ::testing::Test { ...@@ -302,6 +312,9 @@ class WebApkInstallerTest : public ::testing::Test {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
// Sets the Web Manifest's start URL.
void SetStartUrl(const GURL& start_url) { start_url_ = start_url; }
// Sets the best Web Manifest's primary icon URL. // Sets the best Web Manifest's primary icon URL.
void SetBestPrimaryIconUrl(const GURL& best_primary_icon_url) { void SetBestPrimaryIconUrl(const GURL& best_primary_icon_url) {
best_primary_icon_url_ = best_primary_icon_url; best_primary_icon_url_ = best_primary_icon_url;
...@@ -330,8 +343,8 @@ class WebApkInstallerTest : public ::testing::Test { ...@@ -330,8 +343,8 @@ class WebApkInstallerTest : public ::testing::Test {
void SetSpaceStatus(const SpaceStatus status) { test_space_status_ = status; } void SetSpaceStatus(const SpaceStatus status) { test_space_status_ = status; }
std::unique_ptr<WebApkInstallerRunner> CreateWebApkInstallerRunner() { std::unique_ptr<WebApkInstallerRunner> CreateWebApkInstallerRunner() {
return std::unique_ptr<WebApkInstallerRunner>( return std::unique_ptr<WebApkInstallerRunner>(new WebApkInstallerRunner(
new WebApkInstallerRunner(profile_.get(), best_primary_icon_url_, profile_.get(), start_url_, best_primary_icon_url_,
best_badge_icon_url_, test_space_status_)); best_badge_icon_url_, test_space_status_));
} }
...@@ -344,6 +357,7 @@ class WebApkInstallerTest : public ::testing::Test { ...@@ -344,6 +357,7 @@ class WebApkInstallerTest : public ::testing::Test {
private: private:
// Sets default configuration for running WebApkInstaller. // Sets default configuration for running WebApkInstaller.
void SetDefaults() { void SetDefaults() {
SetStartUrl(test_server_.GetURL(kStartUrl));
SetBestPrimaryIconUrl(test_server_.GetURL(kBestPrimaryIconUrl)); SetBestPrimaryIconUrl(test_server_.GetURL(kBestPrimaryIconUrl));
SetBestBadgeIconUrl(test_server_.GetURL(kBestBadgeIconUrl)); SetBestBadgeIconUrl(test_server_.GetURL(kBestBadgeIconUrl));
SetWebApkServerUrl(test_server_.GetURL(kServerUrl)); SetWebApkServerUrl(test_server_.GetURL(kServerUrl));
...@@ -362,6 +376,9 @@ class WebApkInstallerTest : public ::testing::Test { ...@@ -362,6 +376,9 @@ class WebApkInstallerTest : public ::testing::Test {
content::TestBrowserThreadBundle thread_bundle_; content::TestBrowserThreadBundle thread_bundle_;
net::EmbeddedTestServer test_server_; net::EmbeddedTestServer test_server_;
// Web Manifest's start URL.
GURL start_url_;
// Web Manifest's icon URLs. // Web Manifest's icon URLs.
GURL best_primary_icon_url_; GURL best_primary_icon_url_;
GURL best_badge_icon_url_; GURL best_badge_icon_url_;
...@@ -390,6 +407,17 @@ TEST_F(WebApkInstallerTest, FailOnLowSpace) { ...@@ -390,6 +407,17 @@ TEST_F(WebApkInstallerTest, FailOnLowSpace) {
EXPECT_EQ(WebApkInstallResult::FAILURE, runner->result()); EXPECT_EQ(WebApkInstallResult::FAILURE, runner->result());
} }
// Test that installation succeeds when the primary icon is guarded by
// a Cross-Origin-Resource-Policy: same-origin header and the icon is
// same-origin with the start URL.
TEST_F(WebApkInstallerTest, CrossOriginResourcePolicySameOriginIconSuccess) {
SetBestPrimaryIconUrl(test_server()->GetURL(kBestPrimaryIconCorpUrl));
std::unique_ptr<WebApkInstallerRunner> runner = CreateWebApkInstallerRunner();
runner->RunInstallWebApk();
EXPECT_EQ(WebApkInstallResult::SUCCESS, runner->result());
}
// Test that installation fails if fetching the bitmap at the best primary icon // Test that installation fails if fetching the bitmap at the best primary icon
// URL returns no content. In a perfect world the fetch would always succeed // URL returns no content. In a perfect world the fetch would always succeed
// because the fetch for the same icon succeeded recently. // because the fetch for the same icon succeeded recently.
......
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "ui/gfx/android/java_bitmap.h" #include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/codec/png_codec.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h"
using base::android::JavaParamRef; using base::android::JavaParamRef;
using base::android::ScopedJavaLocalRef; using base::android::ScopedJavaLocalRef;
...@@ -160,7 +161,7 @@ void WebApkUpdateDataFetcher::OnDidGetInstallableData( ...@@ -160,7 +161,7 @@ void WebApkUpdateDataFetcher::OnDidGetInstallableData(
content::BrowserContext::GetDefaultStoragePartition(profile) content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess() ->GetURLLoaderFactoryForBrowserProcess()
.get(), .get(),
info_.best_primary_icon_url, url::Origin::Create(last_fetched_url_), info_.best_primary_icon_url,
base::Bind(&WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash, base::Bind(&WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
} }
...@@ -179,7 +180,7 @@ void WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash( ...@@ -179,7 +180,7 @@ void WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash(
content::BrowserContext::GetDefaultStoragePartition(profile) content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess() ->GetURLLoaderFactoryForBrowserProcess()
.get(), .get(),
info_.best_badge_icon_url, url::Origin::Create(last_fetched_url_), info_.best_badge_icon_url,
base::Bind(&WebApkUpdateDataFetcher::OnDataAvailable, base::Bind(&WebApkUpdateDataFetcher::OnDataAvailable,
weak_ptr_factory_.GetWeakPtr(), primary_icon_murmur2_hash, weak_ptr_factory_.GetWeakPtr(), primary_icon_murmur2_hash,
true)); true));
......
HTTP/1.1 200 OK
Content-Type: image/png
Cross-Origin-Resource-Policy: same-origin
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