Changing SandboxedUnpacker image sanitization.
In preparation for moving extension unpacking to the browser process,
making the image sanitization (decoding/reencoding) be triggered by
SandboxedUnpacker in the browser process, using the new ImageSanitizer
class. ImageSanitizer decodes images safely using the data decoder
service and then reencodes them.
Bug: 800540
Change-Id: I40edae3cbd4ab8f03a2494a8d615107bc7feff6d
Tbr: [email protected], [email protected]
Reviewed-on: https://chromium-review.googlesource.com/870744
Reviewed-by: Ken Rockot <[email protected]>
Reviewed-by: Tom Sepez <[email protected]>
Reviewed-by: James Cook <[email protected]>
Reviewed-by: Jay Civelli <[email protected]>
Reviewed-by: Scott Violet <[email protected]>
Reviewed-by: Reilly Grant <[email protected]>
Commit-Queue: Jay Civelli <[email protected]>
Cr-Commit-Position: refs/heads/master@{#531440}diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 39a69ca..029d661 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -4792,6 +4792,7 @@
"//components/drive:test_support",
"//components/storage_monitor:test_support",
"//extensions:test_support",
+ "//services/data_decoder/public/cpp:test_support",
]
}
diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc
index 875188f5..65b5ee1 100644
--- a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc
+++ b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc
@@ -25,6 +25,7 @@
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/image_loader.h"
#include "extensions/browser/sandboxed_unpacker.h"
@@ -35,6 +36,7 @@
#include "extensions/common/manifest_handlers/icons_handler.h"
#include "extensions/common/manifest_handlers/kiosk_mode_info.h"
#include "services/network/public/interfaces/url_loader_factory.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/image/image.h"
@@ -85,8 +87,12 @@
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) {}
void Start() {
+ auto connector = content::ServiceManagerConnection::GetForProcess()
+ ->GetConnector()
+ ->Clone();
task_runner_->PostTask(FROM_HERE,
- base::BindOnce(&CrxLoader::StartInThreadPool, this));
+ base::BindOnce(&CrxLoader::StartInThreadPool, this,
+ std::move(connector)));
}
bool success() const { return success_; }
@@ -130,7 +136,8 @@
NotifyFinishedInThreadPool();
}
- void StartInThreadPool() {
+ void StartInThreadPool(
+ std::unique_ptr<service_manager::Connector> connector) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (!temp_dir_.CreateUniqueTempDir()) {
@@ -139,10 +146,10 @@
return;
}
- scoped_refptr<extensions::SandboxedUnpacker> unpacker(
- new extensions::SandboxedUnpacker(
- extensions::Manifest::INTERNAL, extensions::Extension::NO_FLAGS,
- temp_dir_.GetPath(), task_runner_.get(), this));
+ auto unpacker = base::MakeRefCounted<extensions::SandboxedUnpacker>(
+ std::move(connector), extensions::Manifest::INTERNAL,
+ extensions::Extension::NO_FLAGS, temp_dir_.GetPath(),
+ task_runner_.get(), this);
unpacker->StartWithCrx(extensions::CRXFileInfo(crx_file_));
}
diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc
index 6a7d64e5..bac4700 100644
--- a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc
+++ b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc
@@ -7,8 +7,10 @@
#include "base/bind.h"
#include "base/location.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest_constants.h"
+#include "services/service_manager/public/cpp/connector.h"
namespace chromeos {
@@ -27,10 +29,12 @@
}
void KioskExternalUpdateValidator::Start() {
- scoped_refptr<extensions::SandboxedUnpacker> unpacker(
- new extensions::SandboxedUnpacker(
- extensions::Manifest::EXTERNAL_PREF, extensions::Extension::NO_FLAGS,
- crx_unpack_dir_, backend_task_runner_.get(), this));
+ auto unpacker = base::MakeRefCounted<extensions::SandboxedUnpacker>(
+ content::ServiceManagerConnection::GetForProcess()
+ ->GetConnector()
+ ->Clone(),
+ extensions::Manifest::EXTERNAL_PREF, extensions::Extension::NO_FLAGS,
+ crx_unpack_dir_, backend_task_runner_.get(), this);
if (!backend_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&extensions::SandboxedUnpacker::StartWithCrx,
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 666b488..65713040 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -40,6 +40,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_file_task_runner.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
@@ -62,6 +63,7 @@
#include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/user_script.h"
#include "extensions/strings/grit/extensions_strings.h"
+#include "services/service_manager/public/cpp/connector.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/l10n/l10n_util.h"
@@ -100,6 +102,9 @@
}
// static
+service_manager::Connector* CrxInstaller::connector_for_test_ = nullptr;
+
+// static
scoped_refptr<CrxInstaller> CrxInstaller::Create(
ExtensionService* service,
std::unique_ptr<ExtensionInstallPrompt> client,
@@ -185,8 +190,8 @@
source_file_ = source_file.path;
auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
- install_source_, creation_flags_, install_directory_,
- installer_task_runner_.get(), this);
+ GetConnector()->Clone(), install_source_, creation_flags_,
+ install_directory_, installer_task_runner_.get(), this);
if (!installer_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&SandboxedUnpacker::StartWithCrx, unpacker,
@@ -207,8 +212,8 @@
source_file_ = unpacked_dir;
auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
- install_source_, creation_flags_, install_directory_,
- installer_task_runner_.get(), this);
+ GetConnector()->Clone(), install_source_, creation_flags_,
+ install_directory_, installer_task_runner_.get(), this);
if (!installer_task_runner_->PostTask(
FROM_HERE,
@@ -1026,4 +1031,11 @@
}
}
+service_manager::Connector* CrxInstaller::GetConnector() const {
+ return connector_for_test_
+ ? connector_for_test_
+ : content::ServiceManagerConnection::GetForProcess()
+ ->GetConnector();
+}
+
} // namespace extensions
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index d14ea61..17205c97 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -37,6 +37,10 @@
class SequencedTaskRunner;
}
+namespace service_manager {
+class Connector;
+}
+
namespace extensions {
class CrxInstallError;
class ExtensionUpdaterTest;
@@ -237,6 +241,10 @@
// invalid if this isn't an update.
const base::Version& current_version() const { return current_version_; }
+ static void set_connector_for_test(service_manager::Connector* connector) {
+ connector_for_test_ = connector;
+ }
+
private:
friend class ::ExtensionServiceTest;
friend class ExtensionUpdaterTest;
@@ -307,6 +315,9 @@
// and needs additional permissions.
void ConfirmReEnable();
+ // Returns the connector to the ServiceManager.
+ service_manager::Connector* GetConnector() const;
+
void set_install_flag(int flag, bool val) {
if (val)
install_flags_ |= flag;
@@ -484,6 +495,8 @@
// Invoked when the install is completed.
InstallerResultCallback installer_callback_;
+ static service_manager::Connector* connector_for_test_;
+
DISALLOW_COPY_AND_ASSIGN(CrxInstaller);
};
diff --git a/chrome/browser/extensions/extension_service_test_base.cc b/chrome/browser/extensions/extension_service_test_base.cc
index 09b97fd9..69426a8 100644
--- a/chrome/browser/extensions/extension_service_test_base.cc
+++ b/chrome/browser/extensions/extension_service_test_base.cc
@@ -15,6 +15,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "chrome/browser/extensions/component_loader.h"
+#include "chrome/browser/extensions/crx_installer.h"
#include "chrome/browser/extensions/extension_garbage_collector_factory.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/load_error_reporter.h"
@@ -30,6 +31,7 @@
#include "components/sync_preferences/pref_service_mock_factory.h"
#include "components/sync_preferences/pref_service_syncable.h"
#include "content/public/browser/browser_context.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/pref_names.h"
@@ -91,6 +93,7 @@
return;
}
data_dir_ = test_data_dir.AppendASCII("extensions");
+ CrxInstaller::set_connector_for_test(test_data_decoder_service_.connector());
}
ExtensionServiceTestBase::~ExtensionServiceTestBase() {
diff --git a/chrome/browser/extensions/extension_service_test_base.h b/chrome/browser/extensions/extension_service_test_base.h
index e90e534..0d210e6 100644
--- a/chrome/browser/extensions/extension_service_test_base.h
+++ b/chrome/browser/extensions/extension_service_test_base.h
@@ -19,6 +19,7 @@
#include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/test_renderer_host.h"
#include "content/public/test/test_utils.h"
+#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_CHROMEOS)
@@ -181,6 +182,10 @@
chromeos::ScopedTestUserManager test_user_manager_;
#endif
+ // An instance of the data decoder service that does not require the
+ // ServiceManager.
+ data_decoder::TestDataDecoderService test_data_decoder_service_;
+
DISALLOW_COPY_AND_ASSIGN(ExtensionServiceTestBase);
};
diff --git a/chrome/browser/extensions/startup_helper.cc b/chrome/browser/extensions/startup_helper.cc
index 8e7f342..bb99318 100644
--- a/chrome/browser/extensions/startup_helper.cc
+++ b/chrome/browser/extensions/startup_helper.cc
@@ -17,9 +17,11 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/chrome_extensions_client.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_file_task_runner.h"
#include "extensions/browser/sandboxed_unpacker.h"
#include "extensions/common/extension.h"
+#include "services/service_manager/public/cpp/connector.h"
using content::BrowserThread;
@@ -86,13 +88,17 @@
quit_closure_(std::move(quit_closure)),
success_(false) {}
- bool success() { return success_; }
- const base::string16& error() { return error_; }
+ bool success() const { return success_; }
+ const base::string16& error() const { return error_; }
void Start() {
+ std::unique_ptr<::service_manager::Connector> connector =
+ content::ServiceManagerConnection::GetForProcess()
+ ->GetConnector()
+ ->Clone();
GetExtensionFileTaskRunner()->PostTask(
- FROM_HERE,
- base::BindOnce(&ValidateCrxHelper::StartOnBlockingThread, this));
+ FROM_HERE, base::BindOnce(&ValidateCrxHelper::StartOnBlockingThread,
+ this, std::move(connector)));
}
protected:
@@ -126,11 +132,13 @@
std::move(quit_closure_).Run();
}
- void StartOnBlockingThread() {
+ void StartOnBlockingThread(
+ std::unique_ptr<service_manager::Connector> connector) {
DCHECK(GetExtensionFileTaskRunner()->RunsTasksInCurrentSequence());
- scoped_refptr<SandboxedUnpacker> unpacker(new SandboxedUnpacker(
- Manifest::INTERNAL, 0, /* no special creation flags */
- temp_dir_, GetExtensionFileTaskRunner().get(), this));
+ auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
+ std::move(connector), Manifest::INTERNAL,
+ 0, /* no special creation flags */
+ temp_dir_, GetExtensionFileTaskRunner().get(), this);
unpacker->StartWithCrx(crx_file_);
}
@@ -173,8 +181,8 @@
base::RunLoop run_loop;
CRXFileInfo file(path);
- scoped_refptr<ValidateCrxHelper> helper(
- new ValidateCrxHelper(file, temp_dir.GetPath(), run_loop.QuitClosure()));
+ auto helper = base::MakeRefCounted<ValidateCrxHelper>(
+ file, temp_dir.GetPath(), run_loop.QuitClosure());
helper->Start();
run_loop.Run();
diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc
index beaa4f8a..dbd097f 100644
--- a/chrome/browser/extensions/test_extension_system.cc
+++ b/chrome/browser/extensions/test_extension_system.cc
@@ -10,6 +10,7 @@
#include "base/memory/ptr_util.h"
#include "chrome/browser/extensions/blacklist.h"
#include "chrome/browser/extensions/chrome_app_sorting.h"
+#include "chrome/browser/extensions/crx_installer.h"
#include "chrome/browser/extensions/extension_management.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/shared_module_service.h"
@@ -17,6 +18,7 @@
#include "chrome/common/chrome_switches.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
@@ -72,6 +74,11 @@
profile_, command_line, install_directory, ExtensionPrefs::Get(profile_),
Blacklist::Get(profile_), autoupdate_enabled, extensions_enabled,
&ready_));
+ if (!test_data_decoder_service_) {
+ test_data_decoder_service_ =
+ std::make_unique<data_decoder::TestDataDecoderService>();
+ }
+ CrxInstaller::set_connector_for_test(test_data_decoder_service_->connector());
extension_service_->ClearProvidersForTesting();
return extension_service_.get();
}
diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h
index 73b4e6d..3a51811b 100644
--- a/chrome/browser/extensions/test_extension_system.h
+++ b/chrome/browser/extensions/test_extension_system.h
@@ -9,6 +9,7 @@
#include "extensions/browser/extension_system.h"
#include "extensions/common/one_shot_event.h"
+#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
#if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/login/users/scoped_test_user_manager.h"
@@ -99,6 +100,8 @@
std::unique_ptr<QuotaService> quota_service_;
std::unique_ptr<AppSorting> app_sorting_;
OneShotEvent ready_;
+ std::unique_ptr<data_decoder::TestDataDecoderService>
+ test_data_decoder_service_;
#if defined(OS_CHROMEOS)
std::unique_ptr<chromeos::ScopedTestUserManager> test_user_manager_;
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index 7a106d31..24f41ae 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -254,6 +254,8 @@
"image_loader.h",
"image_loader_factory.cc",
"image_loader_factory.h",
+ "image_sanitizer.cc",
+ "image_sanitizer.h",
"info_map.cc",
"info_map.h",
"install_flag.h",
@@ -550,6 +552,7 @@
"file_highlighter_unittest.cc",
"file_reader_unittest.cc",
"image_loader_unittest.cc",
+ "image_sanitizer_unittest.cc",
"info_map_unittest.cc",
"lazy_background_task_queue_unittest.cc",
"load_monitoring_extension_host_queue_unittest.cc",
diff --git a/extensions/browser/DEPS b/extensions/browser/DEPS
index bde65cb..64eb946 100644
--- a/extensions/browser/DEPS
+++ b/extensions/browser/DEPS
@@ -23,7 +23,7 @@
"+net",
# This directory contains build flags and does not pull all of PPAPI in.
"+ppapi/features",
- "+services/data_decoder/public/cpp",
+ "+services/data_decoder/public",
"+services/network/public/cpp",
"+services/network/public/interfaces",
"+services/preferences/public/cpp",
diff --git a/extensions/browser/image_sanitizer.cc b/extensions/browser/image_sanitizer.cc
new file mode 100644
index 0000000..886ad701
--- /dev/null
+++ b/extensions/browser/image_sanitizer.cc
@@ -0,0 +1,220 @@
+// Copyright 2018 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 "extensions/browser/image_sanitizer.h"
+
+#include "base/files/file_util.h"
+#include "base/task_runner_util.h"
+#include "extensions/browser/extension_file_task_runner.h"
+#include "extensions/common/extension_resource_path_normalizer.h"
+#include "services/service_manager/public/cpp/connector.h"
+#include "ui/gfx/codec/png_codec.h"
+
+namespace extensions {
+
+namespace {
+
+// We don't expect icons and other extension's images to be big.
+// We use this limit to prevent from opening too large images.
+const int kMaxImageCanvas = 4096 * 4096; // 16MB
+
+// Reads the file in |path| and then deletes it.
+// Returns a tuple containing: the file content, whether the read was
+// successful, whether the delete was successful.
+std::tuple<std::vector<uint8_t>, bool, bool> ReadAndDeleteFile(
+ const base::FilePath& path) {
+ std::vector<uint8_t> contents;
+ bool read_success = false;
+ int64_t file_size;
+ if (base::GetFileSize(path, &file_size)) {
+ contents.resize(file_size);
+ read_success = base::ReadFile(
+ path, reinterpret_cast<char*>(contents.data()), file_size);
+ }
+ bool delete_success = base::DeleteFile(path, /*recursive=*/false);
+ return std::make_tuple(std::move(contents), read_success, delete_success);
+}
+
+std::pair<bool, std::vector<unsigned char>> EncodeImage(const SkBitmap& image) {
+ std::vector<unsigned char> image_data;
+ bool success = gfx::PNGCodec::EncodeBGRASkBitmap(
+ image,
+ /*discard_transparency=*/false, &image_data);
+ return std::make_pair(success, std::move(image_data));
+}
+
+int WriteFile(const base::FilePath& path,
+ const std::vector<unsigned char>& data) {
+ return base::WriteFile(path, reinterpret_cast<const char*>(data.data()),
+ base::checked_cast<int>(data.size()));
+}
+
+} // namespace
+
+// static
+std::unique_ptr<ImageSanitizer> ImageSanitizer::CreateAndStart(
+ service_manager::Connector* connector,
+ const service_manager::Identity& identity,
+ const base::FilePath& image_dir,
+ const std::set<base::FilePath>& image_paths,
+ ImageDecodedCallback image_decoded_callback,
+ SanitizationDoneCallback done_callback) {
+ std::unique_ptr<ImageSanitizer> sanitizer(new ImageSanitizer(
+ image_dir, image_paths, std::move(image_decoded_callback),
+ std::move(done_callback)));
+ sanitizer->Start(connector, identity);
+ return sanitizer;
+}
+
+ImageSanitizer::ImageSanitizer(
+ const base::FilePath& image_dir,
+ const std::set<base::FilePath>& image_relative_paths,
+ ImageDecodedCallback image_decoded_callback,
+ SanitizationDoneCallback done_callback)
+ : image_dir_(image_dir),
+ image_paths_(image_relative_paths),
+ image_decoded_callback_(std::move(image_decoded_callback)),
+ done_callback_(std::move(done_callback)),
+ weak_factory_(this) {}
+
+ImageSanitizer::~ImageSanitizer() = default;
+
+void ImageSanitizer::Start(service_manager::Connector* connector,
+ const service_manager::Identity& identity) {
+ if (image_paths_.empty()) {
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::BindOnce(&ImageSanitizer::ReportSuccess,
+ weak_factory_.GetWeakPtr()));
+ return;
+ }
+
+ connector->BindInterface(identity, &image_decoder_ptr_);
+
+ std::set<base::FilePath> normalized_image_paths;
+ for (const base::FilePath& path : image_paths_) {
+ // Normalize paths as |image_paths_| can contain duplicates like "icon.png"
+ // and "./icon.png" to avoid unpacking the same image twice.
+ base::FilePath normalized_path;
+ if (path.IsAbsolute() || path.ReferencesParent() ||
+ !NormalizeExtensionResourcePath(path, &normalized_path)) {
+ // Report the error asynchronously so the caller stack has chance to
+ // unwind.
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::BindOnce(&ImageSanitizer::ReportError,
+ weak_factory_.GetWeakPtr(),
+ Status::kImagePathError, path));
+ return;
+ }
+ normalized_image_paths.insert(normalized_path);
+ }
+ // Update |image_paths_| as some of the path might have been changed by
+ // normalization.
+ image_paths_ = std::move(normalized_image_paths);
+
+ // Note that we use 2 for loops instead of one to prevent a race and flakyness
+ // in tests: if |image_paths_| contains 2 paths, a valid one that points to a
+ // file that does not exist and an invalid one, there is a race that can cause
+ // either error to be reported (kImagePathError or kFileReadError).
+ for (const base::FilePath& path : image_paths_) {
+ base::FilePath full_image_path = image_dir_.Append(path);
+ base::PostTaskAndReplyWithResult(
+ extensions::GetExtensionFileTaskRunner().get(), FROM_HERE,
+ base::BindOnce(&ReadAndDeleteFile, full_image_path),
+ base::BindOnce(&ImageSanitizer::ImageFileRead,
+ weak_factory_.GetWeakPtr(), path));
+ }
+}
+
+void ImageSanitizer::ImageFileRead(
+ const base::FilePath& image_path,
+ std::tuple<std::vector<uint8_t>, bool, bool> read_and_delete_result) {
+ if (!std::get<1>(read_and_delete_result)) {
+ ReportError(Status::kFileReadError, image_path);
+ return;
+ }
+ if (!std::get<2>(read_and_delete_result)) {
+ ReportError(Status::kFileDeleteError, image_path);
+ return;
+ }
+ const std::vector<uint8_t>& image_data = std::get<0>(read_and_delete_result);
+ image_decoder_ptr_->DecodeImage(
+ image_data, data_decoder::mojom::ImageCodec::DEFAULT,
+ /*shrink_to_fit=*/false, kMaxImageCanvas, gfx::Size(),
+ base::BindOnce(&ImageSanitizer::ImageDecoded, weak_factory_.GetWeakPtr(),
+ image_path));
+}
+
+void ImageSanitizer::ImageDecoded(const base::FilePath& image_path,
+ const SkBitmap& decoded_image) {
+ if (decoded_image.isNull()) {
+ ReportError(Status::kDecodingError, image_path);
+ return;
+ }
+
+ if (image_decoded_callback_)
+ image_decoded_callback_.Run(image_path, decoded_image);
+
+ // TODO(mpcomplete): It's lame that we're encoding all images as PNG, even
+ // though they may originally be .jpg, etc. Figure something out.
+ // http://code.google.com/p/chromium/issues/detail?id=12459
+ base::PostTaskAndReplyWithResult(
+ extensions::GetExtensionFileTaskRunner().get(), FROM_HERE,
+ base::BindOnce(&EncodeImage, decoded_image),
+ base::BindOnce(&ImageSanitizer::ImageReencoded,
+ weak_factory_.GetWeakPtr(), image_path));
+}
+
+void ImageSanitizer::ImageReencoded(
+ const base::FilePath& image_path,
+ std::pair<bool, std::vector<unsigned char>> result) {
+ bool success = result.first;
+ std::vector<unsigned char> image_data = std::move(result.second);
+ if (!success) {
+ ReportError(Status::kEncodingError, image_path);
+ return;
+ }
+
+ int size = base::checked_cast<int>(image_data.size());
+ base::PostTaskAndReplyWithResult(
+ extensions::GetExtensionFileTaskRunner().get(), FROM_HERE,
+ base::BindOnce(&WriteFile, image_dir_.Append(image_path),
+ std::move(image_data)),
+ base::BindOnce(&ImageSanitizer::ImageWritten, weak_factory_.GetWeakPtr(),
+ image_path, size));
+}
+
+void ImageSanitizer::ImageWritten(const base::FilePath& image_path,
+ int expected_size,
+ int actual_size) {
+ if (expected_size != actual_size) {
+ ReportError(Status::kFileWriteError, image_path);
+ return;
+ }
+ // We have finished with this path.
+ size_t removed_count = image_paths_.erase(image_path);
+ DCHECK_EQ(1U, removed_count);
+
+ if (image_paths_.empty()) {
+ // This was the last path, we are done.
+ ReportSuccess();
+ }
+}
+
+void ImageSanitizer::ReportSuccess() {
+ CleanUp();
+ std::move(done_callback_).Run(Status::kSuccess, base::FilePath());
+}
+
+void ImageSanitizer::ReportError(Status status, const base::FilePath& path) {
+ CleanUp();
+ // Prevent any other task from reporting, we want to notify only once.
+ weak_factory_.InvalidateWeakPtrs();
+ std::move(done_callback_).Run(status, path);
+}
+
+void ImageSanitizer::CleanUp() {
+ image_decoder_ptr_.reset();
+}
+
+} // namespace extensions
\ No newline at end of file
diff --git a/extensions/browser/image_sanitizer.h b/extensions/browser/image_sanitizer.h
new file mode 100644
index 0000000..ac36aa6
--- /dev/null
+++ b/extensions/browser/image_sanitizer.h
@@ -0,0 +1,116 @@
+// Copyright 2018 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.
+
+#ifndef EXTENSIONS_BROWSER_IMAGE_SANITIZER_H_
+#define EXTENSIONS_BROWSER_IMAGE_SANITIZER_H_
+
+#include <cstdint>
+#include <memory>
+#include <set>
+#include <tuple>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/values.h"
+#include "services/data_decoder/public/interfaces/image_decoder.mojom.h"
+#include "services/service_manager/public/cpp/identity.h"
+
+class SkBitmap;
+
+namespace service_manager {
+class Connector;
+}
+
+namespace extensions {
+
+// This class takes potentially unsafe images and decodes them in a sandboxed
+// process, then reencodes them so that they can later be safely used in the
+// browser process.
+class ImageSanitizer {
+ public:
+ enum class Status {
+ kSuccess = 0,
+ kImagePathError,
+ kFileReadError,
+ kFileDeleteError,
+ kDecodingError,
+ kEncodingError,
+ kFileWriteError,
+ };
+
+ // Callback invoked when the image sanitization is is done. If status is an
+ // error, |path| points to the file that caused the error.
+ using SanitizationDoneCallback =
+ base::OnceCallback<void(Status status, const base::FilePath& path)>;
+
+ // Called on a background thread when an image has been decoded.
+ using ImageDecodedCallback =
+ base::RepeatingCallback<void(const base::FilePath& path, SkBitmap image)>;
+
+ // Creates an ImageSanitizer and starts the sanitization of the images in
+ // |image_relative_paths|. These paths should be relative and not reference
+ // their parent dir or an kImagePathError will be reported to |done_callback|.
+ // These relative paths are resolved against |image_dir|.
+ // |connector| should be a connector to the ServiceManager usable on the
+ // current thread. |identity| is used when accessing the data decoder service
+ // which is used internally to decode images. It lets callers potentially
+ // share a process when doing unrelated data decoding operations.
+ // |done_callback| is invoked asynchronously when all images have been
+ // sanitized or if an error occurred.
+ // If the returned ImageSanitizer instance is deleted, |done_callback| and
+ // |image_decoded_callback| are not called and the sanitization stops promptly
+ // (some background tasks may still run).
+ static std::unique_ptr<ImageSanitizer> CreateAndStart(
+ service_manager::Connector* connector,
+ const service_manager::Identity& identity,
+ const base::FilePath& image_dir,
+ const std::set<base::FilePath>& image_relative_paths,
+ ImageDecodedCallback image_decoded_callback,
+ SanitizationDoneCallback done_callback);
+
+ ~ImageSanitizer();
+
+ private:
+ ImageSanitizer(const base::FilePath& image_dir,
+ const std::set<base::FilePath>& image_relative_paths,
+ ImageDecodedCallback image_decoded_callback,
+ SanitizationDoneCallback done_callback);
+
+ void Start(service_manager::Connector* connector,
+ const service_manager::Identity& identity);
+
+ void ImageFileRead(
+ const base::FilePath& image_path,
+ std::tuple<std::vector<uint8_t>, bool, bool> read_and_delete_result);
+
+ void ImageDecoded(const base::FilePath& image_path,
+ const SkBitmap& decoded_image);
+
+ void ImageReencoded(const base::FilePath& image_path,
+ std::pair<bool, std::vector<unsigned char>> result);
+
+ void ImageWritten(const base::FilePath& image_path,
+ int expected_size,
+ int actual_size);
+
+ void ReportSuccess();
+ void ReportError(Status status, const base::FilePath& path);
+
+ void CleanUp();
+
+ base::FilePath image_dir_;
+ std::set<base::FilePath> image_paths_;
+ ImageDecodedCallback image_decoded_callback_;
+ SanitizationDoneCallback done_callback_;
+ data_decoder::mojom::ImageDecoderPtr image_decoder_ptr_;
+ base::WeakPtrFactory<ImageSanitizer> weak_factory_;
+
+ DISALLOW_COPY_AND_ASSIGN(ImageSanitizer);
+};
+
+} // namespace extensions
+
+#endif // EXTENSIONS_BROWSER_IMAGE_SANITIZER_H_
\ No newline at end of file
diff --git a/extensions/browser/image_sanitizer_unittest.cc b/extensions/browser/image_sanitizer_unittest.cc
new file mode 100644
index 0000000..27eecb3
--- /dev/null
+++ b/extensions/browser/image_sanitizer_unittest.cc
@@ -0,0 +1,211 @@
+// Copyright 2018 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 "extensions/browser/image_sanitizer.h"
+
+#include <map>
+#include <memory>
+
+#include "base/base64.h"
+#include "base/bind.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "build/build_config.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+namespace {
+
+constexpr char kBase64edValidPng[] =
+ "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd"
+ "1PeAAAADElEQVQI12P4//8/AAX+Av7czFnnAAAAAElFTkSuQmCC";
+
+constexpr char kBase64edInvalidPng[] =
+ "Rw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd"
+ "1PeAAAADElEQVQI12P4//8/AAX+Av7czFnnAAAAAElFTkSuQmCC";
+
+class ImageSanitizerTest : public testing::Test {
+ public:
+ ImageSanitizerTest()
+ : thread_bundle_(content::TestBrowserThreadBundle::DEFAULT) {}
+
+ protected:
+ void CreateValidImage(const base::FilePath::StringPieceType& file_name) {
+ ASSERT_TRUE(WriteBase64DataToFile(kBase64edValidPng, file_name));
+ }
+
+ void CreateInvalidImage(const base::FilePath::StringPieceType& file_name) {
+ ASSERT_TRUE(WriteBase64DataToFile(kBase64edInvalidPng, file_name));
+ }
+
+ const base::FilePath& GetImagePath() const { return temp_dir_.GetPath(); }
+
+ void WaitForSanitizationDone() {
+ ASSERT_FALSE(done_callback_);
+ base::RunLoop run_loop;
+ done_callback_ = run_loop.QuitClosure();
+ run_loop.Run();
+ }
+
+ void CreateAndStartSanitizer(
+ const std::set<base::FilePath>& image_relative_paths) {
+ sanitizer_ = ImageSanitizer::CreateAndStart(
+ test_data_decoder_service_.connector(), service_manager::Identity(),
+ temp_dir_.GetPath(), image_relative_paths,
+ base::BindRepeating(&ImageSanitizerTest::ImageSanitizerDecodedImage,
+ base::Unretained(this)),
+ base::BindOnce(&ImageSanitizerTest::ImageSanitizationDone,
+ base::Unretained(this)));
+ }
+
+ ImageSanitizer::Status last_reported_status() const { return last_status_; }
+
+ const base::FilePath& last_reported_path() const {
+ return last_reported_path_;
+ }
+
+ std::map<base::FilePath, SkBitmap>* decoded_images() {
+ return &decoded_images_;
+ }
+
+ private:
+ bool WriteBase64DataToFile(const std::string& base64_data,
+ const base::FilePath::StringPieceType& file_name) {
+ std::string binary;
+ if (!base::Base64Decode(base64_data, &binary))
+ return false;
+
+ base::FilePath path = temp_dir_.GetPath().Append(file_name);
+ return base::WriteFile(path, binary.data(), binary.size());
+ }
+
+ void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+
+ void ImageSanitizationDone(ImageSanitizer::Status status,
+ const base::FilePath& path) {
+ last_status_ = status;
+ last_reported_path_ = path;
+ if (done_callback_)
+ std::move(done_callback_).Run();
+ }
+
+ void ImageSanitizerDecodedImage(const base::FilePath& path, SkBitmap image) {
+ EXPECT_EQ(decoded_images()->find(path), decoded_images()->end());
+ (*decoded_images())[path] = image;
+ }
+
+ content::TestBrowserThreadBundle thread_bundle_;
+ data_decoder::TestDataDecoderService test_data_decoder_service_;
+ ImageSanitizer::Status last_status_;
+ base::FilePath last_reported_path_;
+ base::OnceClosure done_callback_;
+ std::unique_ptr<ImageSanitizer> sanitizer_;
+ base::ScopedTempDir temp_dir_;
+ std::map<base::FilePath, SkBitmap> decoded_images_;
+
+ DISALLOW_COPY_AND_ASSIGN(ImageSanitizerTest);
+};
+
+} // namespace
+
+TEST_F(ImageSanitizerTest, NoImagesProvided) {
+ CreateAndStartSanitizer(std::set<base::FilePath>());
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kSuccess);
+ EXPECT_TRUE(last_reported_path().empty());
+}
+
+TEST_F(ImageSanitizerTest, InvalidPathAbsolute) {
+ base::FilePath normal_path(FILE_PATH_LITERAL("hello.png"));
+#if defined(OS_WIN)
+ base::FilePath absolute_path(FILE_PATH_LITERAL("c:\\Windows\\win32"));
+#else
+ base::FilePath absolute_path(FILE_PATH_LITERAL("/usr/bin/root"));
+#endif
+ CreateAndStartSanitizer({normal_path, absolute_path});
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kImagePathError);
+ EXPECT_EQ(last_reported_path(), absolute_path);
+}
+
+TEST_F(ImageSanitizerTest, InvalidPathReferenceParent) {
+ base::FilePath good_path(FILE_PATH_LITERAL("hello.png"));
+ base::FilePath bad_path(FILE_PATH_LITERAL("hello"));
+ bad_path = bad_path.Append(base::FilePath::kParentDirectory)
+ .Append(base::FilePath::kParentDirectory)
+ .Append(base::FilePath::kParentDirectory)
+ .Append(FILE_PATH_LITERAL("usr"))
+ .Append(FILE_PATH_LITERAL("bin"));
+ CreateAndStartSanitizer({good_path, bad_path});
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kImagePathError);
+ EXPECT_EQ(last_reported_path(), bad_path);
+}
+
+TEST_F(ImageSanitizerTest, ValidCase) {
+ constexpr std::array<const base::FilePath::CharType* const, 10> kFileNames{
+ {FILE_PATH_LITERAL("image0.png"), FILE_PATH_LITERAL("image1.png"),
+ FILE_PATH_LITERAL("image2.png"), FILE_PATH_LITERAL("image3.png"),
+ FILE_PATH_LITERAL("image4.png"), FILE_PATH_LITERAL("image5.png"),
+ FILE_PATH_LITERAL("image6.png"), FILE_PATH_LITERAL("image7.png"),
+ FILE_PATH_LITERAL("image8.png"), FILE_PATH_LITERAL("image9.png")}};
+ std::set<base::FilePath> paths;
+ for (const base::FilePath::CharType* file_name : kFileNames) {
+ CreateValidImage(file_name);
+ paths.insert(base::FilePath(file_name));
+ }
+ CreateAndStartSanitizer(paths);
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kSuccess);
+ EXPECT_TRUE(last_reported_path().empty());
+ // Make sure the image files are there and non empty, and that the
+ // ImageSanitizerDecodedImage callback was invoked for every image.
+ for (const auto& path : paths) {
+ int64_t file_size = 0;
+ base::FilePath full_path = GetImagePath().Append(path);
+ EXPECT_TRUE(base::GetFileSize(full_path, &file_size));
+ EXPECT_GT(file_size, 0);
+
+ ASSERT_TRUE(base::ContainsKey(*decoded_images(), path));
+ EXPECT_FALSE((*decoded_images())[path].drawsNothing());
+ }
+ // No extra images should have been reported.
+ EXPECT_EQ(decoded_images()->size(), 10U);
+}
+
+TEST_F(ImageSanitizerTest, MissingImage) {
+ constexpr base::FilePath::CharType kGoodPngName[] =
+ FILE_PATH_LITERAL("image.png");
+ constexpr base::FilePath::CharType kNonExistingName[] =
+ FILE_PATH_LITERAL("i_don_t_exist.png");
+ CreateValidImage(kGoodPngName);
+ base::FilePath good_png(kGoodPngName);
+ base::FilePath bad_png(kNonExistingName);
+ CreateAndStartSanitizer({good_png, bad_png});
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kFileReadError);
+ EXPECT_EQ(last_reported_path(), bad_png);
+}
+
+TEST_F(ImageSanitizerTest, InvalidImage) {
+ constexpr base::FilePath::CharType kGoodPngName[] =
+ FILE_PATH_LITERAL("good.png");
+ constexpr base::FilePath::CharType kBadPngName[] =
+ FILE_PATH_LITERAL("bad.png");
+ CreateValidImage(kGoodPngName);
+ CreateInvalidImage(kBadPngName);
+ base::FilePath good_png(kGoodPngName);
+ base::FilePath bad_png(kBadPngName);
+ CreateAndStartSanitizer({good_png, bad_png});
+ WaitForSanitizationDone();
+ EXPECT_EQ(last_reported_status(), ImageSanitizer::Status::kDecodingError);
+ EXPECT_EQ(last_reported_path(), bad_png);
+}
+
+} // namespace extensions
diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc
index 5aa7aa9e..84cdd3d 100644
--- a/extensions/browser/sandboxed_unpacker.cc
+++ b/extensions/browser/sandboxed_unpacker.cc
@@ -15,6 +15,7 @@
#include "base/bind.h"
#include "base/command_line.h"
#include "base/files/file_util.h"
+#include "base/i18n/rtl.h"
#include "base/json/json_string_value_serializer.h"
#include "base/metrics/histogram_macros.h"
#include "base/path_service.h"
@@ -41,6 +42,8 @@
#include "extensions/common/manifest_handlers/icons_handler.h"
#include "extensions/common/switches.h"
#include "extensions/strings/grit/extensions_strings.h"
+#include "services/data_decoder/public/interfaces/constants.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/codec/png_codec.h"
@@ -182,21 +185,6 @@
return false;
}
-// Read the decoded images back from the file we saved them to.
-// |extension_path| is the path to the extension we unpacked that wrote the
-// data. Returns true on success.
-bool ReadImagesFromFile(const base::FilePath& extension_path,
- DecodedImages* images) {
- base::FilePath path = extension_path.AppendASCII(kDecodedImagesFilename);
- std::string file_str;
- if (!base::ReadFileToString(path, &file_str))
- return false;
-
- IPC::Message pickle(file_str.data(), file_str.size());
- base::PickleIterator iter(pickle);
- return IPC::ReadParam(&pickle, &iter, images);
-}
-
// Read the decoded message catalogs back from the file we saved them to.
// |extension_path| is the path to the extension we unpacked that wrote the
// data. Returns true on success.
@@ -223,12 +211,14 @@
}
SandboxedUnpacker::SandboxedUnpacker(
+ std::unique_ptr<service_manager::Connector> connector,
Manifest::Location location,
int creation_flags,
const base::FilePath& extensions_dir,
const scoped_refptr<base::SequencedTaskRunner>& unpacker_io_task_runner,
SandboxedUnpackerClient* client)
- : client_(client),
+ : connector_(std::move(connector)),
+ client_(client),
extensions_dir_(extensions_dir),
location_(location),
creation_flags_(creation_flags),
@@ -237,6 +227,12 @@
// the utility process kills itself for a bad IPC.
CHECK_GT(location, Manifest::INVALID_LOCATION);
CHECK_LT(location, Manifest::NUM_LOCATIONS);
+
+ // Use a random instance ID to guarantee the connection is to a new data
+ // decoder service (running in its own process).
+ data_decoder_identity_ = service_manager::Identity(
+ data_decoder::mojom::kServiceName, service_manager::mojom::kInheritUserID,
+ base::UnguessableToken::Create().ToString());
}
bool SandboxedUnpacker::CreateTempDirectory() {
@@ -501,21 +497,105 @@
return;
}
- SkBitmap install_icon;
- if (!RewriteImageFiles(&install_icon))
+ // The install icon path may be empty, which is OK, but if it is not it should
+ // be normalized successfully.
+ const std::string& original_install_icon_path =
+ IconsInfo::GetIcons(extension_.get())
+ .Get(extension_misc::EXTENSION_ICON_LARGE,
+ ExtensionIconSet::MATCH_BIGGER);
+ if (!original_install_icon_path.empty() &&
+ !NormalizeExtensionResourcePath(
+ base::FilePath::FromUTF8Unsafe(original_install_icon_path),
+ &install_icon_path_)) {
+ // Invalid path for browser image.
+ ReportFailure(INVALID_PATH_FOR_BROWSER_IMAGE,
+ l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("INVALID_PATH_FOR_BROWSER_IMAGE")));
return;
-
- if (!RewriteCatalogFiles())
- return;
-
- // Index and persist ruleset for the Declarative Net Request API.
- base::Optional<int> dnr_ruleset_checksum;
- if (!IndexAndPersistRulesIfNeeded(std::move(json_ruleset),
- &dnr_ruleset_checksum)) {
- return; // Failure was already reported.
}
- ReportSuccess(std::move(manifest), install_icon, dnr_ruleset_checksum);
+ std::set<base::FilePath> image_paths =
+ ExtensionsClient::Get()->GetBrowserImagePaths(extension_.get());
+ image_sanitizer_ = ImageSanitizer::CreateAndStart(
+ connector_.get(), data_decoder_identity_, extension_root_, image_paths,
+ base::BindRepeating(&SandboxedUnpacker::ImageSanitizerDecodedImage, this),
+ base::BindOnce(&SandboxedUnpacker::ImageSanitizationDone, this,
+ std::move(manifest), std::move(json_ruleset)));
+}
+
+void SandboxedUnpacker::ImageSanitizationDone(
+ std::unique_ptr<base::DictionaryValue> manifest,
+ std::unique_ptr<base::ListValue> json_ruleset,
+ ImageSanitizer::Status status,
+ const base::FilePath& file_path_for_error) {
+ // Release |image_sanitizer_|, it is not useful anymore and reference |this|.
+ // (keep a local ref to this so if |image_sanitizer_| was the last ref to
+ // |this| it stays valid for the duration of the call).
+ scoped_refptr<SandboxedUnpacker> keep_this_alive = this;
+ image_sanitizer_ = nullptr;
+
+ if (status == ImageSanitizer::Status::kSuccess) {
+ if (!RewriteCatalogFiles())
+ return;
+
+ // Index and persist ruleset for the Declarative Net Request API.
+ base::Optional<int> dnr_ruleset_checksum;
+ if (!IndexAndPersistRulesIfNeeded(std::move(json_ruleset),
+ &dnr_ruleset_checksum)) {
+ return; // Failure was already reported.
+ }
+
+ ReportSuccess(std::move(manifest), dnr_ruleset_checksum);
+ return;
+ }
+
+ FailureReason failure_reason = UNPACKER_CLIENT_FAILED;
+ base::string16 error;
+ switch (status) {
+ case ImageSanitizer::Status::kImagePathError:
+ failure_reason = INVALID_PATH_FOR_BROWSER_IMAGE;
+ error = l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("INVALID_PATH_FOR_BROWSER_IMAGE"));
+ break;
+ case ImageSanitizer::Status::kFileReadError:
+ case ImageSanitizer::Status::kDecodingError:
+ error = l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_IMAGE_ERROR,
+ base::i18n::GetDisplayStringInLTRDirectionality(
+ file_path_for_error.BaseName().LossyDisplayName()));
+ break;
+ case ImageSanitizer::Status::kFileDeleteError:
+ failure_reason = ERROR_REMOVING_OLD_IMAGE_FILE;
+ error = l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("ERROR_REMOVING_OLD_IMAGE_FILE"));
+ break;
+ case ImageSanitizer::Status::kEncodingError:
+ failure_reason = ERROR_RE_ENCODING_THEME_IMAGE;
+ error = l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("ERROR_RE_ENCODING_THEME_IMAGE"));
+ break;
+ case ImageSanitizer::Status::kFileWriteError:
+ failure_reason = ERROR_SAVING_THEME_IMAGE;
+ error =
+ l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("ERROR_SAVING_THEME_IMAGE"));
+ break;
+ default:
+ NOTREACHED();
+ break;
+ }
+
+ ReportFailure(failure_reason, error);
+}
+
+void SandboxedUnpacker::ImageSanitizerDecodedImage(const base::FilePath& path,
+ SkBitmap image) {
+ if (path == install_icon_path_)
+ install_icon_ = image;
}
bool SandboxedUnpacker::IndexAndPersistRulesIfNeeded(
@@ -754,7 +834,6 @@
void SandboxedUnpacker::ReportSuccess(
std::unique_ptr<base::DictionaryValue> original_manifest,
- const SkBitmap& install_icon,
const base::Optional<int>& dnr_ruleset_checksum) {
UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1);
@@ -767,7 +846,7 @@
// Client takes ownership of temporary directory, manifest, and extension.
client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_,
std::move(original_manifest), extension_.get(),
- install_icon, dnr_ruleset_checksum);
+ install_icon_, dnr_ruleset_checksum);
extension_ = NULL;
}
@@ -806,131 +885,6 @@
return final_manifest.release();
}
-bool SandboxedUnpacker::RewriteImageFiles(SkBitmap* install_icon) {
- DCHECK(!temp_dir_.GetPath().empty());
-
- DecodedImages images;
- if (!ReadImagesFromFile(temp_dir_.GetPath(), &images)) {
- // Couldn't read image data from disk.
- ReportFailure(COULD_NOT_READ_IMAGE_DATA_FROM_DISK,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("COULD_NOT_READ_IMAGE_DATA_FROM_DISK")));
- return false;
- }
-
- // Delete any images that may be used by the browser. We're going to write
- // out our own versions of the parsed images, and we want to make sure the
- // originals are gone for good.
- std::set<base::FilePath> image_paths =
- ExtensionsClient::Get()->GetBrowserImagePaths(extension_.get());
-
- // Decoded |images| set contains normalized paths and |image_paths| contains
- // original paths. It is required to check sizes of normalized sets.
- std::set<base::FilePath> normalized_image_paths =
- NormalizeExtensionResourcePaths(image_paths);
- if (normalized_image_paths.size() != images.size()) {
- // Decoded images don't match what's in the manifest.
- ReportFailure(
- DECODED_IMAGES_DO_NOT_MATCH_THE_MANIFEST,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("DECODED_IMAGES_DO_NOT_MATCH_THE_MANIFEST")));
- return false;
- }
-
- // Report if original icons paths invalid. Normalization procedure may skip
- // invalid paths, so it is required to check exactly original paths.
- for (const auto& path : image_paths) {
- if (path.IsAbsolute() || path.ReferencesParent()) {
- // Invalid path for browser image.
- ReportFailure(INVALID_PATH_FOR_BROWSER_IMAGE,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("INVALID_PATH_FOR_BROWSER_IMAGE")));
- return false;
- }
- }
-
- for (const auto& path : normalized_image_paths) {
- if (!base::DeleteFile(extension_root_.Append(path), false)) {
- // Error removing old image file.
- ReportFailure(ERROR_REMOVING_OLD_IMAGE_FILE,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("ERROR_REMOVING_OLD_IMAGE_FILE")));
- return false;
- }
- }
-
- // Get install icon normalized path. Original install icon path may be empty
- // and it is ok. But if original install icon path is not empty, it should be
- // normalized successfully.
- const std::string& install_icon_path =
- IconsInfo::GetIcons(extension_.get())
- .Get(extension_misc::EXTENSION_ICON_LARGE,
- ExtensionIconSet::MATCH_BIGGER);
- base::FilePath normalized_install_icon_path;
- if (!install_icon_path.empty() &&
- !NormalizeExtensionResourcePath(
- base::FilePath::FromUTF8Unsafe(install_icon_path),
- &normalized_install_icon_path)) {
- // Invalid path for browser image.
- ReportFailure(INVALID_PATH_FOR_BROWSER_IMAGE,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("INVALID_PATH_FOR_BROWSER_IMAGE")));
- return false;
- }
-
- // Write our parsed images back to disk as well.
- for (size_t i = 0; i < images.size(); ++i) {
- const SkBitmap& image = std::get<0>(images[i]);
- const base::FilePath& path_suffix = std::get<1>(images[i]);
- if (path_suffix == normalized_install_icon_path)
- *install_icon = image;
-
- if (path_suffix.IsAbsolute() || path_suffix.ReferencesParent()) {
- // Invalid path for bitmap image.
- ReportFailure(INVALID_PATH_FOR_BITMAP_IMAGE,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("INVALID_PATH_FOR_BITMAP_IMAGE")));
- return false;
- }
-
- base::FilePath path = extension_root_.Append(path_suffix);
-
- std::vector<unsigned char> image_data;
- // TODO(mpcomplete): It's lame that we're encoding all images as PNG, even
- // though they may originally be .jpg, etc. Figure something out.
- // http://code.google.com/p/chromium/issues/detail?id=12459
- if (!gfx::PNGCodec::EncodeBGRASkBitmap(image, false, &image_data)) {
- // Error re-encoding theme image.
- ReportFailure(ERROR_RE_ENCODING_THEME_IMAGE,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("ERROR_RE_ENCODING_THEME_IMAGE")));
- return false;
- }
-
- // Note: we're overwriting existing files that the utility process wrote,
- // so we can be sure the directory exists.
- const char* image_data_ptr = reinterpret_cast<const char*>(&image_data[0]);
- int size = base::checked_cast<int>(image_data.size());
- if (base::WriteFile(path, image_data_ptr, size) != size) {
- // Error saving theme image.
- ReportFailure(
- ERROR_SAVING_THEME_IMAGE,
- l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("ERROR_SAVING_THEME_IMAGE")));
- return false;
- }
- }
-
- return true;
-}
-
bool SandboxedUnpacker::RewriteCatalogFiles() {
base::DictionaryValue catalogs;
if (!ReadMessageCatalogsFromFile(temp_dir_.GetPath(), &catalogs)) {
diff --git a/extensions/browser/sandboxed_unpacker.h b/extensions/browser/sandboxed_unpacker.h
index a8fe285..a9dd506 100644
--- a/extensions/browser/sandboxed_unpacker.h
+++ b/extensions/browser/sandboxed_unpacker.h
@@ -17,8 +17,10 @@
#include "base/time/time.h"
#include "content/public/browser/utility_process_mojo_client.h"
#include "extensions/browser/crx_file_info.h"
+#include "extensions/browser/image_sanitizer.h"
#include "extensions/browser/install/crx_install_error.h"
#include "extensions/common/manifest.h"
+#include "services/service_manager/public/cpp/identity.h"
class SkBitmap;
@@ -28,6 +30,10 @@
class SequencedTaskRunner;
}
+namespace service_manager {
+class Connector;
+}
+
namespace extensions {
class Extension;
@@ -101,6 +107,8 @@
// passing the |location| and |creation_flags| to Extension::Create. The
// |extensions_dir| parameter should specify the directory under which we'll
// create a subdirectory to write the unpacked extension contents.
+ // |connector| must be a fresh connector (not yet associated to any thread) to
+ // the service manager.
// Note: Because this requires disk I/O, the task runner passed should use
// TaskShutdownBehavior::SKIP_ON_SHUTDOWN to ensure that either the task is
// fully run (if initiated before shutdown) or not run at all (if shutdown is
@@ -110,6 +118,7 @@
// TODO(devlin): SKIP_ON_SHUTDOWN is also not quite sufficient for this. We
// should probably instead be using base::ImportantFileWriter or similar.
SandboxedUnpacker(
+ std::unique_ptr<service_manager::Connector> connector,
Manifest::Location location,
int creation_flags,
const base::FilePath& extensions_dir,
@@ -236,9 +245,14 @@
std::unique_ptr<base::ListValue> json_ruleset);
void UnpackExtensionFailed(const base::string16& error);
+ void ImageSanitizationDone(std::unique_ptr<base::DictionaryValue> manifest,
+ std::unique_ptr<base::ListValue> json_ruleset,
+ ImageSanitizer::Status status,
+ const base::FilePath& path);
+ void ImageSanitizerDecodedImage(const base::FilePath& path, SkBitmap image);
+
// Reports unpack success or failure, or unzip failure.
void ReportSuccess(std::unique_ptr<base::DictionaryValue> original_manifest,
- const SkBitmap& install_icon,
const base::Optional<int>& dnr_ruleset_checksum);
void ReportFailure(FailureReason reason, const base::string16& error);
@@ -249,7 +263,6 @@
// Overwrites original files with safe results from utility process.
// Reports error and returns false if it fails.
- bool RewriteImageFiles(SkBitmap* install_icon);
bool RewriteCatalogFiles();
// Cleans up temp directory artifacts.
@@ -263,6 +276,9 @@
std::unique_ptr<base::ListValue> json_ruleset,
base::Optional<int>* dnr_ruleset_checksum);
+ // Connector to the ServiceManager required by the Unzip API.
+ std::unique_ptr<service_manager::Connector> connector_;
+
// If we unpacked a CRX file, we hold on to the path name for use
// in various histograms.
base::FilePath crx_path_for_histograms_;
@@ -307,6 +323,21 @@
std::unique_ptr<content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>
utility_process_mojo_client_;
+ // The normalized path of the install icon path, retrieved from the manifest.
+ base::FilePath install_icon_path_;
+
+ // The decoded install icon.
+ SkBitmap install_icon_;
+
+ // The identity used to connect to the data decoder service. It is unique to
+ // this SandboxedUnpacker instance so that data decoder operations for
+ // unpacking this extension share the same process, and so that no unrelated
+ // data decoder operation use that process.
+ service_manager::Identity data_decoder_identity_;
+
+ // The ImageSanitizer used to clean-up images.
+ std::unique_ptr<ImageSanitizer> image_sanitizer_;
+
DISALLOW_COPY_AND_ASSIGN(SandboxedUnpacker);
};
diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc
index 5e27fe165..40f1508 100644
--- a/extensions/browser/sandboxed_unpacker_unittest.cc
+++ b/extensions/browser/sandboxed_unpacker_unittest.cc
@@ -12,6 +12,7 @@
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/values.h"
#include "components/crx_file/id_util.h"
@@ -23,12 +24,45 @@
#include "extensions/common/extension.h"
#include "extensions/common/extension_paths.h"
#include "extensions/common/switches.h"
+#include "extensions/test/test_extensions_client.h"
+#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
+#include "services/service_manager/public/cpp/connector.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/zlib/google/zip.h"
namespace extensions {
+namespace {
+
+// Inserts an illegal path into the browser images returned by
+// TestExtensionsClient for any extension.
+class IllegalImagePathInserter
+ : public TestExtensionsClient::BrowserImagePathsFilter {
+ public:
+ IllegalImagePathInserter(TestExtensionsClient* client) : client_(client) {
+ client_->AddBrowserImagePathsFilter(this);
+ }
+
+ virtual ~IllegalImagePathInserter() {
+ client_->RemoveBrowserImagePathsFilter(this);
+ }
+
+ void Filter(const Extension* extension,
+ std::set<base::FilePath>* paths) override {
+ base::FilePath illegal_path =
+ base::FilePath(base::FilePath::kParentDirectory)
+ .AppendASCII(kTempExtensionName)
+ .AppendASCII("product_logo_128.png");
+ paths->insert(illegal_path);
+ }
+
+ private:
+ TestExtensionsClient* client_;
+};
+
+} // namespace
+
class MockSandboxedUnpackerClient : public SandboxedUnpackerClient {
public:
void WaitForUnpack() {
@@ -83,7 +117,8 @@
client_ = new MockSandboxedUnpackerClient;
sandboxed_unpacker_ = new SandboxedUnpacker(
- Manifest::INTERNAL, Extension::NO_FLAGS, extensions_dir_.GetPath(),
+ test_data_decoder_service_.connector()->Clone(), Manifest::INTERNAL,
+ Extension::NO_FLAGS, extensions_dir_.GetPath(),
base::ThreadTaskRunnerHandle::Get(), client_);
}
@@ -145,13 +180,16 @@
sandboxed_unpacker_));
}
- base::FilePath GetInstallPath() {
+ bool InstallSucceeded() const { return !client_->temp_dir().empty(); }
+
+ base::FilePath GetInstallPath() const {
return client_->temp_dir().AppendASCII(kTempExtensionName);
}
- base::string16 GetInstallError() { return client_->unpack_err(); }
+ base::string16 GetInstallError() const { return client_->unpack_err(); }
protected:
+ data_decoder::TestDataDecoderService test_data_decoder_service_;
base::ScopedTempDir extensions_dir_;
MockSandboxedUnpackerClient* client_;
scoped_refptr<SandboxedUnpacker> sandboxed_unpacker_;
@@ -159,6 +197,24 @@
in_process_utility_thread_helper_;
};
+TEST_F(SandboxedUnpackerTest, ImageDecodingError) {
+ const char kExpected[] = "Could not decode image: ";
+ SetupUnpacker("bad_image.crx", "");
+ EXPECT_TRUE(base::StartsWith(GetInstallError(), base::ASCIIToUTF16(kExpected),
+ base::CompareCase::INSENSITIVE_ASCII))
+ << "Expected prefix: \"" << kExpected << "\", actual error: \""
+ << GetInstallError() << "\"";
+}
+
+TEST_F(SandboxedUnpackerTest, BadPathError) {
+ IllegalImagePathInserter inserter(
+ static_cast<TestExtensionsClient*>(ExtensionsClient::Get()));
+ SetupUnpacker("good_package.crx", "");
+ // Install should have failed with an error.
+ EXPECT_FALSE(InstallSucceeded());
+ EXPECT_FALSE(GetInstallError().empty());
+}
+
TEST_F(SandboxedUnpackerTest, NoCatalogsSuccess) {
SetupUnpacker("no_l10n.crx", "");
// Check that there is no _locales folder.
diff --git a/extensions/common/constants.cc b/extensions/common/constants.cc
index ef80d21..57b6606 100644
--- a/extensions/common/constants.cc
+++ b/extensions/common/constants.cc
@@ -29,8 +29,6 @@
const char kTempExtensionName[] = "CRX_INSTALL";
-const char kDecodedImagesFilename[] = "DECODED_IMAGES";
-
const char kDecodedMessageCatalogsFilename[] = "DECODED_MESSAGE_CATALOGS";
const char kGeneratedBackgroundPageFilename[] =
diff --git a/extensions/common/constants.h b/extensions/common/constants.h
index a55a018..f0d6c6ffb 100644
--- a/extensions/common/constants.h
+++ b/extensions/common/constants.h
@@ -46,9 +46,6 @@
// validation before finalizing install.
extern const char kTempExtensionName[];
-// The file to write our decoded images to, relative to the extension_path.
-extern const char kDecodedImagesFilename[];
-
// The file to write our decoded message catalogs to, relative to the
// extension_path.
extern const char kDecodedMessageCatalogsFilename[];
diff --git a/extensions/common/extension_unpacker.mojom b/extensions/common/extension_unpacker.mojom
index 46b2a74e..2ce9482 100644
--- a/extensions/common/extension_unpacker.mojom
+++ b/extensions/common/extension_unpacker.mojom
@@ -21,8 +21,8 @@
// the Declarative Net Request API in |json_ruleset|. The supplied |location|,
// and the |creation_flags| defined by Extension::InitFromValueFlags are
// passed into Extension::Create() when unpacking the extension. Decoded
- // image and message catalog data from the extension is written to files
- // kDecodedImagesFilename and kDecodedMessageCatalogsFilename in |path|.
+ // message catalog data from the extension is written to the
+ // kDecodedMessageCatalogsFilename file in |path|.
// If Unpack() fails for any reason, |error| contains a user-displayable
// explanation of what went wrong.
// |channel| and |type| are needed to initialize the global state of the
diff --git a/extensions/utility/unpacker.cc b/extensions/utility/unpacker.cc
index c43edf7..405bf335 100644
--- a/extensions/utility/unpacker.cc
+++ b/extensions/utility/unpacker.cc
@@ -20,12 +20,10 @@
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread.h"
#include "base/values.h"
-#include "content/public/child/image_decoder_utils.h"
#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_l10n_util.h"
-#include "extensions/common/extension_resource_path_normalizer.h"
#include "extensions/common/extension_utility_types.h"
#include "extensions/common/extensions_client.h"
#include "extensions/common/file_util.h"
@@ -46,54 +44,12 @@
namespace errors = manifest_errors;
namespace keys = manifest_keys;
-// A limit to stop us passing dangerously large canvases to the browser.
-const int kMaxImageCanvas = 4096 * 4096;
-
constexpr const base::FilePath::CharType* kAllowedThemeFiletypes[] = {
FILE_PATH_LITERAL(".bmp"), FILE_PATH_LITERAL(".gif"),
FILE_PATH_LITERAL(".jpeg"), FILE_PATH_LITERAL(".jpg"),
FILE_PATH_LITERAL(".json"), FILE_PATH_LITERAL(".png"),
FILE_PATH_LITERAL(".webp")};
-SkBitmap DecodeImage(const base::FilePath& path) {
- // Read the file from disk.
- std::string file_contents;
- if (!base::PathExists(path) ||
- !base::ReadFileToString(path, &file_contents)) {
- return SkBitmap();
- }
-
- // Decode the image using WebKit's image decoder.
- const unsigned char* data =
- reinterpret_cast<const unsigned char*>(file_contents.data());
- SkBitmap bitmap =
- content::DecodeImage(data, gfx::Size(), file_contents.length());
- if (bitmap.computeByteSize() > kMaxImageCanvas)
- return SkBitmap();
- return bitmap;
-}
-
-bool PathContainsParentDirectory(const base::FilePath& path) {
- const base::FilePath::StringType kSeparators(base::FilePath::kSeparators);
- const base::FilePath::StringType kParentDirectory(
- base::FilePath::kParentDirectory);
- const size_t npos = base::FilePath::StringType::npos;
- const base::FilePath::StringType& value = path.value();
-
- for (size_t i = 0; i < value.length();) {
- i = value.find(kParentDirectory, i);
- if (i != npos) {
- if ((i == 0 || kSeparators.find(value[i - 1]) == npos) &&
- (i + 1 < value.length() || kSeparators.find(value[i + 1]) == npos)) {
- return true;
- }
- ++i;
- }
- }
-
- return false;
-}
-
bool WritePickle(const IPC::Message& pickle, const base::FilePath& dest_path) {
int size = base::checked_cast<int>(pickle.size());
const char* data = static_cast<const char*>(pickle.data());
@@ -103,10 +59,6 @@
} // namespace
-struct Unpacker::InternalData {
- DecodedImages decoded_images;
-};
-
Unpacker::Unpacker(const base::FilePath& working_dir,
const base::FilePath& extension_dir,
const std::string& extension_id,
@@ -116,9 +68,7 @@
extension_dir_(extension_dir),
extension_id_(extension_id),
location_(location),
- creation_flags_(creation_flags) {
- internal_data_.reset(new InternalData());
-}
+ creation_flags_(creation_flags) {}
Unpacker::~Unpacker() {
}
@@ -221,20 +171,6 @@
}
extension->AddInstallWarnings(warnings);
- // Decode any images that the browser needs to display. |GetBrowserImagePaths|
- // can contain duplicates like "icon.png" and "./icon.png". It is required to
- // normalize paths to avoid unpacking of the same icon twice.
- const std::set<base::FilePath> image_paths =
- ExtensionsClient::Get()->GetBrowserImagePaths(extension.get());
- if (!VerifyOriginalImagePaths(image_paths))
- return false; // Error was already reported.
- const std::set<base::FilePath> normalized_image_paths =
- NormalizeExtensionResourcePaths(image_paths);
- for (const base::FilePath& path : normalized_image_paths) {
- if (!AddDecodedImage(path))
- return false; // Error was already reported.
- }
-
// Parse all message catalogs (if any).
parsed_catalogs_.reset(new base::DictionaryValue);
if (!LocaleInfo::GetDefaultLocale(extension.get()).empty()) {
@@ -242,7 +178,7 @@
return false; // Error was already reported.
}
- return ReadJSONRulesetIfNeeded(extension.get()) && DumpImagesToFile() &&
+ return ReadJSONRulesetIfNeeded(extension.get()) &&
DumpMessageCatalogsToFile();
}
@@ -270,19 +206,6 @@
return true;
}
-bool Unpacker::DumpImagesToFile() {
- IPC::Message pickle; // We use a Message so we can use WriteParam.
- IPC::WriteParam(&pickle, internal_data_->decoded_images);
-
- base::FilePath path = working_dir_.AppendASCII(kDecodedImagesFilename);
- if (!WritePickle(pickle, path)) {
- SetError("Could not write image data to disk.");
- return false;
- }
-
- return true;
-}
-
bool Unpacker::DumpMessageCatalogsToFile() {
IPC::Message pickle;
IPC::WriteParam(&pickle, *parsed_catalogs_.get());
@@ -297,35 +220,6 @@
return true;
}
-bool Unpacker::AddDecodedImage(const base::FilePath& path) {
- SkBitmap image_bitmap = DecodeImage(extension_dir_.Append(path));
- if (image_bitmap.isNull()) {
- SetUTF16Error(l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_IMAGE_ERROR,
- base::i18n::GetDisplayStringInLTRDirectionality(
- path.BaseName().LossyDisplayName())));
- return false;
- }
-
- internal_data_->decoded_images.push_back(std::make_tuple(image_bitmap, path));
- return true;
-}
-
-bool Unpacker::VerifyOriginalImagePaths(
- const std::set<base::FilePath>& image_paths) {
- for (const base::FilePath& path : image_paths) {
- // Make sure it's not referencing a file outside the extension's subdir.
- if (path.IsAbsolute() || PathContainsParentDirectory(path)) {
- SetUTF16Error(l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_IMAGE_PATH_ERROR,
- base::i18n::GetDisplayStringInLTRDirectionality(
- path.LossyDisplayName())));
- return false;
- }
- }
- return true;
-}
-
bool Unpacker::ReadMessageCatalog(const base::FilePath& message_path) {
std::string error;
JSONFileValueDeserializer deserializer(message_path);
diff --git a/extensions/utility/unpacker.h b/extensions/utility/unpacker.h
index ec9b2fa..4473110f 100644
--- a/extensions/utility/unpacker.h
+++ b/extensions/utility/unpacker.h
@@ -6,7 +6,6 @@
#define EXTENSIONS_UTILITY_UNPACKER_H_
#include <memory>
-#include <set>
#include <string>
#include <vector>
@@ -47,8 +46,7 @@
std::string* error);
// Runs the processing steps for the extension. On success, this returns true
- // and the decoded images will be in a file at
- // |working_dir|/kDecodedImagesFilename and the decoded messages will be in a
+ // and the decoded messages will be in a
// file at |working_dir|/kDecodedMessageCatalogsFilename.
bool Run();
@@ -70,11 +68,6 @@
// error.
bool ReadJSONRulesetIfNeeded(const Extension* extension);
- // Write the decoded images to kDecodedImagesFilename. We do this instead
- // of sending them over IPC, since they are so large. Returns true on
- // success.
- bool DumpImagesToFile();
-
// Write the decoded messages to kDecodedMessageCatalogsFilename. We do this
// instead of sending them over IPC, since they are so large. Returns true on
// success.
@@ -87,10 +80,6 @@
// images.
bool AddDecodedImage(const base::FilePath& path);
- // Verifies original image paths from extension. Returns true iff all paths
- // are valid. Sets an error if at least one path is invalid.
- bool VerifyOriginalImagePaths(const std::set<base::FilePath>& image_paths);
-
// Parses the catalog at the given path and puts it in our list of parsed
// catalogs.
bool ReadMessageCatalog(const base::FilePath& message_path);
@@ -121,11 +110,6 @@
// extension did not provide one.
std::unique_ptr<base::ListValue> parsed_json_ruleset_;
- // A list of decoded images and the paths where those images came from. Paths
- // are relative to the manifest file.
- struct InternalData;
- std::unique_ptr<InternalData> internal_data_;
-
// Dictionary of relative paths and catalogs per path. Paths are in the form
// of _locales/locale, without messages.json base part.
std::unique_ptr<base::DictionaryValue> parsed_catalogs_;
diff --git a/extensions/utility/unpacker_unittest.cc b/extensions/utility/unpacker_unittest.cc
index 1e31a68..fdd085e 100644
--- a/extensions/utility/unpacker_unittest.cc
+++ b/extensions/utility/unpacker_unittest.cc
@@ -141,61 +141,6 @@
EXPECT_EQ(0U, unpacker_->parsed_catalogs()->size());
}
-namespace {
-
-// Inserts an illegal path into the browser images returned by
-// TestExtensionsClient for any extension.
-class IllegalImagePathInserter
- : public TestExtensionsClient::BrowserImagePathsFilter {
- public:
- IllegalImagePathInserter(TestExtensionsClient* client) : client_(client) {
- client_->AddBrowserImagePathsFilter(this);
- }
-
- virtual ~IllegalImagePathInserter() {
- client_->RemoveBrowserImagePathsFilter(this);
- }
-
- void Filter(const Extension* extension,
- std::set<base::FilePath>* paths) override {
- base::FilePath illegal_path =
- base::FilePath(base::FilePath::kParentDirectory)
- .AppendASCII(kTempExtensionName)
- .AppendASCII("product_logo_128.png");
- paths->insert(illegal_path);
- }
-
- private:
- TestExtensionsClient* client_;
-};
-
-} // namespace
-
-TEST_F(UnpackerTest, BadPathError) {
- const char kExpected[] = "Illegal path (absolute or relative with '..'): ";
- SetupUnpacker("good_package.crx");
- IllegalImagePathInserter inserter(
- static_cast<TestExtensionsClient*>(ExtensionsClient::Get()));
-
- EXPECT_FALSE(unpacker_->Run());
- EXPECT_TRUE(base::StartsWith(unpacker_->error_message(),
- ASCIIToUTF16(kExpected),
- base::CompareCase::INSENSITIVE_ASCII))
- << "Expected prefix: \"" << kExpected << "\", actual error: \""
- << unpacker_->error_message() << "\"";
-}
-
-TEST_F(UnpackerTest, ImageDecodingError) {
- const char kExpected[] = "Could not decode image: ";
- SetupUnpacker("bad_image.crx");
- EXPECT_FALSE(unpacker_->Run());
- EXPECT_TRUE(base::StartsWith(unpacker_->error_message(),
- ASCIIToUTF16(kExpected),
- base::CompareCase::INSENSITIVE_ASCII))
- << "Expected prefix: \"" << kExpected << "\", actual error: \""
- << unpacker_->error_message() << "\"";
-}
-
struct UnzipFileFilterTestCase {
const base::FilePath::CharType* input;
const bool should_unzip;
