Return an empty mapping for an empty file asset (flutter#10815)

Fixes flutter/flutter#36574
TheDauntless · Aug 13, 2019 · 7711efb · 7711efb
1 parent 663f9a9
commit 7711efb
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 4 deletions.
diff --git a/assets/directory_asset_bundle.cc b/assets/directory_asset_bundle.cc
@@ -38,7 +38,7 @@ std::unique_ptr<fml::Mapping> DirectoryAssetBundle::GetAsMapping(
   auto mapping = std::make_unique<fml::FileMapping>(fml::OpenFile(
       descriptor_, asset_name.c_str(), false, fml::FilePermission::kRead));
 
-  if (mapping->GetMapping() == nullptr) {
+  if (!mapping->IsValid()) {
     return nullptr;
   }
 

diff --git a/fml/file_unittest.cc b/fml/file_unittest.cc
@@ -155,3 +155,17 @@ TEST(FileTest, AtomicWriteTest) {
   // Cleanup.
   ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "precious_data"));
 }
+
+TEST(FileTest, EmptyMappingTest) {
+  fml::ScopedTemporaryDirectory dir;
+
+  auto file = fml::OpenFile(dir.fd(), "my_contents", true,
+                            fml::FilePermission::kReadWrite);
+
+  fml::FileMapping mapping(file);
+  ASSERT_TRUE(mapping.IsValid());
+  ASSERT_EQ(mapping.GetSize(), 0ul);
+  ASSERT_EQ(mapping.GetMapping(), nullptr);
+
+  ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
+}
diff --git a/fml/mapping.cc b/fml/mapping.cc
@@ -31,7 +31,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadOnly(
   auto mapping = std::make_unique<FileMapping>(
       base_fd, std::initializer_list<Protection>{Protection::kRead});
 
-  if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) {
+  if (!mapping->IsValid()) {
     return nullptr;
   }
 
@@ -56,7 +56,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadExecute(
       base_fd, std::initializer_list<Protection>{Protection::kRead,
                                                  Protection::kExecute});
 
-  if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) {
+  if (!mapping->IsValid()) {
     return nullptr;
   }
 

diff --git a/fml/mapping.h b/fml/mapping.h
@@ -67,7 +67,10 @@ class FileMapping final : public Mapping {
 
   uint8_t* GetMutableMapping();
 
+  bool IsValid() const;
+
  private:
+  bool valid_ = false;
   size_t size_ = 0;
   uint8_t* mapping_ = nullptr;
   uint8_t* mutable_mapping_ = nullptr;

diff --git a/fml/platform/posix/mapping_posix.cc b/fml/platform/posix/mapping_posix.cc
@@ -63,7 +63,8 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,
     return;
   }
 
-  if (stat_buffer.st_size <= 0) {
+  if (stat_buffer.st_size == 0) {
+    valid_ = true;
     return;
   }
 
@@ -79,6 +80,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,
 
   mapping_ = static_cast<uint8_t*>(mapping);
   size_ = stat_buffer.st_size;
+  valid_ = true;
   if (is_writable) {
     mutable_mapping_ = mapping_;
   }
@@ -98,4 +100,8 @@ const uint8_t* FileMapping::GetMapping() const {
   return mapping_;
 }
 
+bool FileMapping::IsValid() const {
+  return valid_;
+}
+
 }  // namespace fml
diff --git a/fml/platform/win/mapping_win.cc b/fml/platform/win/mapping_win.cc
@@ -54,6 +54,11 @@ FileMapping::FileMapping(const fml::UniqueFD& fd,
     return;
   }
 
+  if (mapping_size == 0) {
+    valid_ = true;
+    return;
+  }
+
   DWORD protect_flags = 0;
   bool read_only = !IsWritable(protections);
 
@@ -90,6 +95,7 @@ FileMapping::FileMapping(const fml::UniqueFD& fd,
 
   mapping_ = mapping;
   size_ = mapping_size;
+  valid_ = true;
   if (IsWritable(protections)) {
     mutable_mapping_ = mapping_;
   }
@@ -109,4 +115,8 @@ const uint8_t* FileMapping::GetMapping() const {
   return mapping_;
 }
 
+bool FileMapping::IsValid() const {
+  return valid_;
+}
+
 }  // namespace fml