Skip to content

GitLab

T
tangled
Commit 7bda2fdf authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot
Browse files
Options

[WebAPK] Rename |data| in the hasher to |unsafe_data|. 

Bug: 1054405
Change-Id: Icaa7bc8aa8ca6159c7ccc8afa541ccbd4441a8f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066759
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Auto-Submit: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: default avatarPeter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748185}
parent b3fe39c2
Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 24 deletions
chrome/browser/android/webapk/webapk_icon_hasher.cc
View file @ 7bda2fdf
......@@ -102,7 +102,7 @@ void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
if (net::DataURL::Parse(icon_url, &mime_type, &char_set, &data) &&
!data.empty()) {
icon.hash = ComputeMurmur2Hash(data);
icon.data = std::move(data);
icon.unsafe_data = std::move(data);
}
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), std::move(icon)));
......@@ -160,8 +160,8 @@ void WebApkIconHasher::OnSimpleLoaderComplete(
// malicious data. Decoding unsanitized bitmap data to an SkBitmap in the
// browser process is a security bug.
Icon icon;
icon.data = std::move(*response_body);
icon.hash = ComputeMurmur2Hash(icon.data);
icon.unsafe_data = std::move(*response_body);
icon.hash = ComputeMurmur2Hash(icon.unsafe_data);
RunCallback(std::move(icon));
}
......
chrome/browser/android/webapk/webapk_icon_hasher.h
View file @ 7bda2fdf
......@@ -29,7 +29,11 @@ class WebApkIconHasher {
public:
// Result struct for holding the downloaded icon data and its hash.
struct Icon {
std::string data;
// The result of fetching the |icon|. This is untrusted data from the web
// and should not be processed or decoded by the browser process.
std::string unsafe_data;
// The murmur2 hash of |unsafe_data|.
std::string hash;
};
......
chrome/browser/android/webapk/webapk_icon_hasher_unittest.cc
View file @ 7bda2fdf
......@@ -125,7 +125,7 @@ TEST_F(WebApkIconHasherTest, Success) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_EQ(kIconMurmur2Hash, runner.icon().hash);
EXPECT_FALSE(runner.icon().data.empty());
EXPECT_FALSE(runner.icon().unsafe_data.empty());
}
TEST_F(WebApkIconHasherTest, DataUri) {
......@@ -135,7 +135,7 @@ TEST_F(WebApkIconHasherTest, DataUri) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_EQ("536500236142107998", runner.icon().hash);
EXPECT_FALSE(runner.icon().data.empty());
EXPECT_FALSE(runner.icon().unsafe_data.empty());
}
TEST_F(WebApkIconHasherTest, MultipleIconUrls) {
......@@ -164,7 +164,7 @@ TEST_F(WebApkIconHasherTest, MultipleIconUrls) {
auto result = runner.RunMultiple(test_url_loader_factory(), {icon_url1});
ASSERT_EQ(result.size(), 1u);
EXPECT_EQ(result[icon_url1.spec()].hash, kIconMurmur2Hash);
EXPECT_FALSE(result[icon_url1.spec()].data.empty());
EXPECT_FALSE(result[icon_url1.spec()].unsafe_data.empty());
}
{
......@@ -172,10 +172,10 @@ TEST_F(WebApkIconHasherTest, MultipleIconUrls) {
runner.RunMultiple(test_url_loader_factory(), {icon_url1, icon_url2});
ASSERT_EQ(result.size(), 2u);
EXPECT_EQ(result[icon_url1.spec()].hash, kIconMurmur2Hash);
EXPECT_FALSE(result[icon_url1.spec()].data.empty());
EXPECT_FALSE(result[icon_url1.spec()].unsafe_data.empty());
EXPECT_EQ(result[icon_url2.spec()].hash, "536500236142107998");
EXPECT_FALSE(result[icon_url2.spec()].data.empty());
EXPECT_FALSE(result[icon_url2.spec()].unsafe_data.empty());
}
}
......@@ -184,7 +184,7 @@ TEST_F(WebApkIconHasherTest, DataUriInvalid) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_TRUE(runner.icon().hash.empty());
EXPECT_TRUE(runner.icon().data.empty());
EXPECT_TRUE(runner.icon().unsafe_data.empty());
}
TEST_F(WebApkIconHasherTest, InvalidUrl) {
......@@ -192,7 +192,7 @@ TEST_F(WebApkIconHasherTest, InvalidUrl) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_TRUE(runner.icon().hash.empty());
EXPECT_TRUE(runner.icon().data.empty());
EXPECT_TRUE(runner.icon().unsafe_data.empty());
}
TEST_F(WebApkIconHasherTest, DownloadTimedOut) {
......@@ -200,7 +200,7 @@ TEST_F(WebApkIconHasherTest, DownloadTimedOut) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_TRUE(runner.icon().hash.empty());
EXPECT_TRUE(runner.icon().data.empty());
EXPECT_TRUE(runner.icon().unsafe_data.empty());
}
// Test that the hash callback is called with an empty string if an HTTP error
......@@ -220,5 +220,5 @@ TEST_F(WebApkIconHasherTest, HTTPError) {
WebApkIconHasherRunner runner;
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_TRUE(runner.icon().hash.empty());
EXPECT_TRUE(runner.icon().data.empty());
EXPECT_TRUE(runner.icon().unsafe_data.empty());
}
chrome/browser/android/webapk/webapk_installer.cc
View file @ 7bda2fdf
......@@ -293,7 +293,7 @@ std::unique_ptr<std::string> BuildProtoInBackground(
if (icon_url == shortcut_info.splash_image_url.spec()) {
if (shortcut_info.splash_image_url !=
shortcut_info.best_primary_icon_url) {
image->set_image_data(it->second.data);
image->set_image_data(it->second.unsafe_data);
}
image->add_usages(webapk::Image::SPLASH_ICON);
image->add_purposes(webapk::Image::ANY);
......@@ -316,10 +316,11 @@ std::unique_ptr<std::string> BuildProtoInBackground(
// Don't move the hash to avoid clearing it in case of duplicates.
shortcut_icon->set_hash(shortcut_hash_it->second.hash);
if (shortcut_hash_it->second.data.size() <= kMaxIconSizeInBytes) {
if (shortcut_hash_it->second.unsafe_data.size() <=
kMaxIconSizeInBytes) {
// Duplicate icons will have an empty |image_data|.
shortcut_icon->set_image_data(
std::move(shortcut_hash_it->second.data));
std::move(shortcut_hash_it->second.unsafe_data));
}
}
}
......
chrome/browser/android/webapk/webapk_installer_unittest.cc
View file @ 7bda2fdf
......@@ -605,7 +605,7 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsAvailable) {
manifest.icons(1).hash());
EXPECT_THAT(manifest.icons(1).usages(),
testing::ElementsAre(webapk::Image::SPLASH_ICON));
EXPECT_EQ(icon_url_to_murmur2_hash[best_splash_icon_url].data,
EXPECT_EQ(icon_url_to_murmur2_hash[best_splash_icon_url].unsafe_data,
manifest.icons(1).image_data());
// Check protobuf fields for unused icon.
......@@ -620,7 +620,7 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsAvailable) {
EXPECT_EQ(manifest.shortcuts(0).icons(0).hash(),
icon_url_to_murmur2_hash[best_shortcut_icon_url].hash);
EXPECT_EQ(manifest.shortcuts(0).icons(0).image_data(),
icon_url_to_murmur2_hash[best_shortcut_icon_url].data);
icon_url_to_murmur2_hash[best_shortcut_icon_url].unsafe_data);
}
// Tests a WebApk install or update request is built properly when the Chrome
......@@ -672,7 +672,7 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoPrimaryIconAndSplashIconSameUrl) {
EXPECT_EQ(manifest.shortcuts(0).icons(0).hash(),
icon_url_to_murmur2_hash[best_icon_url].hash);
EXPECT_EQ(manifest.shortcuts(0).icons(0).image_data(),
icon_url_to_murmur2_hash[best_icon_url].data);
icon_url_to_murmur2_hash[best_icon_url].unsafe_data);
}
TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenWithMultipleShortcuts) {
......@@ -700,14 +700,14 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenWithMultipleShortcuts) {
EXPECT_EQ(manifest.shortcuts(0).icons(0).hash(),
icon_url_to_murmur2_hash[best_shortcut_icon_url1].hash);
EXPECT_EQ(manifest.shortcuts(0).icons(0).image_data(),
icon_url_to_murmur2_hash[best_shortcut_icon_url1].data);
icon_url_to_murmur2_hash[best_shortcut_icon_url1].unsafe_data);
ASSERT_EQ(manifest.shortcuts(1).icons_size(), 1);
EXPECT_EQ(manifest.shortcuts(1).icons(0).src(), best_shortcut_icon_url2);
EXPECT_EQ(manifest.shortcuts(1).icons(0).hash(),
icon_url_to_murmur2_hash[best_shortcut_icon_url2].hash);
EXPECT_EQ(manifest.shortcuts(1).icons(0).image_data(),
icon_url_to_murmur2_hash[best_shortcut_icon_url2].data);
icon_url_to_murmur2_hash[best_shortcut_icon_url2].unsafe_data);
}
TEST_F(WebApkInstallerTest,
......@@ -733,7 +733,7 @@ TEST_F(WebApkInstallerTest,
EXPECT_EQ(manifest.shortcuts(0).icons(0).hash(),
icon_url_to_murmur2_hash[best_shortcut_icon_url].hash);
EXPECT_EQ(manifest.shortcuts(0).icons(0).image_data(),
icon_url_to_murmur2_hash[best_shortcut_icon_url].data);
icon_url_to_murmur2_hash[best_shortcut_icon_url].unsafe_data);
ASSERT_EQ(manifest.shortcuts(1).icons_size(), 1);
EXPECT_EQ(manifest.shortcuts(1).icons(0).src(), best_shortcut_icon_url);
......@@ -777,7 +777,7 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoSplashIconAndShortcutIconSameUrl) {
testing::ElementsAre(webapk::Image::SPLASH_ICON));
EXPECT_TRUE(manifest.icons(1).has_image_data());
EXPECT_EQ(manifest.icons(1).image_data(),
icon_url_to_murmur2_hash[best_icon_url].data);
icon_url_to_murmur2_hash[best_icon_url].unsafe_data);
// Check protobuf fields for unused icon.
EXPECT_EQ(kUnusedIconPath, manifest.icons(2).src());
......@@ -792,5 +792,5 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoSplashIconAndShortcutIconSameUrl) {
icon_url_to_murmur2_hash[best_icon_url].hash);
EXPECT_TRUE(manifest.shortcuts(0).icons(0).has_image_data());
EXPECT_EQ(manifest.shortcuts(0).icons(0).image_data(),
icon_url_to_murmur2_hash[best_icon_url].data);
icon_url_to_murmur2_hash[best_icon_url].unsafe_data);
}
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