From 7a03249b0f949c2a2ea6a9865d878ffaf9db5cae Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Fri, 19 Apr 2019 14:45:10 -0700 Subject: [PATCH] Only allow mappings for ICU initialization. If the mapping callback is not set or it the callback returns invalid data, ICU initialization will be embedder responsibility. This affects all embedders and the following have been audited: Android: Via a symbol mapping. iOS: Via a file mapping. Embedder: Via a file mapping. Fuchsia: Via a VMO mapping Test shells and Flutter tester: Via file mapping with ICU data needing to be next to the executable. --- benchmarking/benchmarking.cc | 3 +- common/settings.h | 6 - fml/file.cc | 11 ++ fml/file.h | 2 + fml/icu_util.cc | 107 ++---------------- fml/icu_util.h | 4 +- shell/common/shell.cc | 10 +- shell/common/switches.cc | 15 ++- .../framework/Source/FlutterDartProject.mm | 7 +- shell/platform/embedder/embedder.cc | 7 +- shell/testing/tester_main.cc | 10 +- .../txt/tests/txt_run_all_unittests.cc | 4 +- 12 files changed, 61 insertions(+), 125 deletions(-) diff --git a/benchmarking/benchmarking.cc b/benchmarking/benchmarking.cc index c8e5af7ea3dd5..1bf605dfbae2d 100644 --- a/benchmarking/benchmarking.cc +++ b/benchmarking/benchmarking.cc @@ -10,7 +10,8 @@ namespace benchmarking { int Main(int argc, char** argv) { benchmark::Initialize(&argc, argv); - fml::icu::InitializeICU("icudtl.dat"); + fml::icu::InitializeICU(fml::FileMapping::CreateReadOnly( + fml::OpenDirectoryOfExecutable(), "icudtl.dat")); ::benchmark::RunSpecifiedBenchmarks(); return 0; } diff --git a/common/settings.h b/common/settings.h index 317884c1757de..c7ee514fbdb7f 100644 --- a/common/settings.h +++ b/common/settings.h @@ -132,12 +132,6 @@ struct Settings { bool verbose_logging = false; std::string log_tag = "flutter"; - // The icu_initialization_required setting does not have a corresponding - // switch because it is intended to be decided during build time, not runtime. - // Some companies apply source modification here because their build system - // brings its own ICU data files. - bool icu_initialization_required = true; - std::string icu_data_path; MappingCallback icu_mapper; // Assets settings diff --git a/fml/file.cc b/fml/file.cc index 8deb76c91da0f..c2a94e304c94e 100644 --- a/fml/file.cc +++ b/fml/file.cc @@ -5,6 +5,7 @@ #include "flutter/fml/file.h" #include "flutter/fml/logging.h" +#include "flutter/fml/paths.h" namespace fml { @@ -58,4 +59,14 @@ ScopedTemporaryDirectory::~ScopedTemporaryDirectory() { } } +fml::UniqueFD OpenDirectoryOfExecutable() { + auto result = paths::GetExecutableDirectoryPath(); + + if (!result.first) { + return {}; + } + + return OpenFile(result.second.c_str(), false, FilePermission::kRead); +} + } // namespace fml diff --git a/fml/file.h b/fml/file.h index 068ddc84c999d..42a007c221079 100644 --- a/fml/file.h +++ b/fml/file.h @@ -46,6 +46,8 @@ fml::UniqueFD OpenDirectory(const fml::UniqueFD& base_directory, bool create_if_necessary, FilePermission permission); +fml::UniqueFD OpenDirectoryOfExecutable(); + fml::UniqueFD Duplicate(fml::UniqueFD::element_type descriptor); bool IsDirectory(const fml::UniqueFD& directory); diff --git a/fml/icu_util.cc b/fml/icu_util.cc index fa0cc02b9c20d..13bed1440b0ef 100644 --- a/fml/icu_util.cc +++ b/fml/icu_util.cc @@ -4,115 +4,24 @@ #include "flutter/fml/icu_util.h" -#include #include -#include "flutter/fml/build_config.h" #include "flutter/fml/logging.h" -#include "flutter/fml/mapping.h" -#include "flutter/fml/native_library.h" -#include "flutter/fml/paths.h" #include "third_party/icu/source/common/unicode/udata.h" namespace fml { namespace icu { -class ICUContext { - public: - ICUContext(const std::string& icu_data_path) : valid_(false) { - valid_ = SetupMapping(icu_data_path) && SetupICU(); +void InitializeICU(std::unique_ptr mapping) { + if (mapping == nullptr || mapping->GetSize() == 0) { + return; } - - ICUContext(std::unique_ptr mapping) : mapping_(std::move(mapping)) { - valid_ = SetupICU(); - } - - ~ICUContext() = default; - - bool SetupMapping(const std::string& icu_data_path) { - // Check if the path exists and it readable directly. - auto fd = - fml::OpenFile(icu_data_path.c_str(), false, fml::FilePermission::kRead); - - // Check the path relative to the current executable. - if (!fd.is_valid()) { - auto directory = fml::paths::GetExecutableDirectoryPath(); - - if (!directory.first) { - return false; - } - - std::string path_relative_to_executable = - paths::JoinPaths({directory.second, icu_data_path}); - - fd = fml::OpenFile(path_relative_to_executable.c_str(), false, - fml::FilePermission::kRead); - } - - if (!fd.is_valid()) { - return false; - } - - std::initializer_list protection = { - fml::FileMapping::Protection::kRead}; - - auto file_mapping = - std::make_unique(fd, std::move(protection)); - - if (file_mapping->GetSize() != 0) { - mapping_ = std::move(file_mapping); - return true; - } - - return false; - } - - bool SetupICU() { - if (GetSize() == 0) { - return false; - } - - UErrorCode err_code = U_ZERO_ERROR; - udata_setCommonData(GetMapping(), &err_code); - return (err_code == U_ZERO_ERROR); - } - - const uint8_t* GetMapping() const { - return mapping_ ? mapping_->GetMapping() : nullptr; - } - - size_t GetSize() const { return mapping_ ? mapping_->GetSize() : 0; } - - bool IsValid() const { return valid_; } - - private: - bool valid_; - std::unique_ptr mapping_; - - FML_DISALLOW_COPY_AND_ASSIGN(ICUContext); -}; - -void InitializeICUOnce(const std::string& icu_data_path) { - static ICUContext* context = new ICUContext(icu_data_path); - FML_CHECK(context->IsValid()) - << "Must be able to initialize the ICU context. Tried: " << icu_data_path; -} - -std::once_flag g_icu_init_flag; -void InitializeICU(const std::string& icu_data_path) { - std::call_once(g_icu_init_flag, - [&icu_data_path]() { InitializeICUOnce(icu_data_path); }); -} - -void InitializeICUFromMappingOnce(std::unique_ptr mapping) { - static ICUContext* context = new ICUContext(std::move(mapping)); - FML_CHECK(context->IsValid()) - << "Unable to initialize the ICU context from a mapping."; -} - -void InitializeICUFromMapping(std::unique_ptr mapping) { + static std::once_flag g_icu_init_flag; std::call_once(g_icu_init_flag, [mapping = std::move(mapping)]() mutable { - InitializeICUFromMappingOnce(std::move(mapping)); + static auto icu_mapping = std::move(mapping); + UErrorCode err_code = U_ZERO_ERROR; + udata_setCommonData(icu_mapping->GetMapping(), &err_code); + FML_CHECK(err_code == U_ZERO_ERROR) << "Must be able to initialize ICU."; }); } diff --git a/fml/icu_util.h b/fml/icu_util.h index 22b7a906efce0..dc1d947e03118 100644 --- a/fml/icu_util.h +++ b/fml/icu_util.h @@ -13,9 +13,7 @@ namespace fml { namespace icu { -void InitializeICU(const std::string& icu_data_path = ""); - -void InitializeICUFromMapping(std::unique_ptr mapping); +void InitializeICU(std::unique_ptr mapping); } // namespace icu } // namespace fml diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 5a33f0d388eed..33e2b48e79493 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -193,14 +193,8 @@ static void PerformInitializationTasks(const Settings& settings) { FML_DLOG(INFO) << "Skia deterministic rendering is enabled."; } - if (settings.icu_initialization_required) { - if (settings.icu_data_path.size() != 0) { - fml::icu::InitializeICU(settings.icu_data_path); - } else if (settings.icu_mapper) { - fml::icu::InitializeICUFromMapping(settings.icu_mapper()); - } else { - FML_DLOG(WARNING) << "Skipping ICU initialization in the shell."; - } + if (settings.icu_mapper) { + fml::icu::InitializeICU(settings.icu_mapper()); } }); } diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 859a798c0fc25..bd54486c68607 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -242,9 +242,20 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { command_line.GetOptionValue(FlagForSwitch(Switch::CacheDirPath), &settings.temp_directory_path); - if (settings.icu_initialization_required) { + { + // ICU from a data file. + std::string icu_data_path; command_line.GetOptionValue(FlagForSwitch(Switch::ICUDataFilePath), - &settings.icu_data_path); + &icu_data_path); + if (icu_data_path.size() > 0) { + settings.icu_mapper = [icu_data_path]() { + return fml::FileMapping::CreateReadOnly(icu_data_path); + }; + } + } + + { + // ICU from a symbol in a dynamic library if (command_line.HasOption(FlagForSwitch(Switch::ICUSymbolPrefix))) { std::string icu_symbol_prefix, native_lib_path; command_line.GetOptionValue(FlagForSwitch(Switch::ICUSymbolPrefix), diff --git a/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm b/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm index 47277007a97c3..3738c05cd4138 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm @@ -59,10 +59,13 @@ // defaults. // Flutter ships the ICU data file in the the bundle of the engine. Look for it there. - if (settings.icu_data_path.size() == 0) { + if (!settings.icu_mapper) { NSString* icuDataPath = [engineBundle pathForResource:@"icudtl" ofType:@"dat"]; if (icuDataPath.length > 0) { - settings.icu_data_path = icuDataPath.UTF8String; + auto icu_data_path = std::string{icuDataPath.UTF8String}; + settings.icu_mapper = [icu_data_path]() { + return fml::FileMapping::CreateReadOnly(icu_data_path); + }; } } diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index e9b50852bb1e5..544b6330be7d3 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -370,7 +370,12 @@ FlutterEngineResult FlutterEngineRun(size_t version, PopulateSnapshotMappingCallbacks(args, settings); - settings.icu_data_path = icu_data_path; + if (!settings.icu_mapper) { + settings.icu_mapper = [icu_data_path]() { + return fml::FileMapping::CreateReadOnly(icu_data_path); + }; + } + settings.assets_path = args->assets_path; if (!flutter::DartVM::IsRunningPrecompiledCode()) { diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index fd729d1f434d2..f882374cd71c2 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -251,8 +251,14 @@ int main(int argc, char* argv[]) { return EXIT_FAILURE; } - if (settings.icu_data_path.size() == 0) { - settings.icu_data_path = "icudtl.dat"; + // Using command line arguments, the user can specify the ICU data as being + // present in either a file or a dynamic library. If no such specification has + // been, default to icudtl.dat. + if (!settings.icu_mapper) { + settings.icu_mapper = []() { + return fml::FileMapping::CreateReadOnly(fml::OpenDirectoryOfExecutable(), + "icudtl.dat"); + }; } // The tools that read logs get confused if there is a log tag specified. diff --git a/third_party/txt/tests/txt_run_all_unittests.cc b/third_party/txt/tests/txt_run_all_unittests.cc index 992a167f0449e..3832bfcb9246f 100644 --- a/third_party/txt/tests/txt_run_all_unittests.cc +++ b/third_party/txt/tests/txt_run_all_unittests.cc @@ -36,7 +36,9 @@ int main(int argc, char** argv) { } FML_DCHECK(txt::GetFontDir().length() > 0); - fml::icu::InitializeICU("icudtl.dat"); + fml::icu::InitializeICU(fml::FileMapping::CreateReadOnly( + fml::OpenDirectoryOfExecutable(), "icudtl.dat")); + SkGraphics::Init(); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();