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

jason-simmons · web-flow · commit 7711efbf062a · 2019-08-13T10:02:59.000-07:00
Fixes flutter/flutter#36574
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

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ std::unique_ptr<fml::Mapping> DirectoryAssetBundle::GetAsMapping(`
`38`	`38`	`auto mapping = std::make_unique<fml::FileMapping>(fml::OpenFile(`
`39`	`39`	`descriptor_, asset_name.c_str(), false, fml::FilePermission::kRead));`
`40`	`40`
`41`		`- if (mapping->GetMapping() == nullptr) {`
	`41`	`+ if (!mapping->IsValid()) {`
`42`	`42`	`return nullptr;`
`43`	`43`	`}`
`44`	`44`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadOnly(`
`31`	`31`	`auto mapping = std::make_unique<FileMapping>(`
`32`	`32`	`base_fd, std::initializer_list<Protection>{Protection::kRead});`
`33`	`33`
`34`		`- if (mapping->GetSize() == 0 \|\| mapping->GetMapping() == nullptr) {`
	`34`	`+ if (!mapping->IsValid()) {`
`35`	`35`	`return nullptr;`
`36`	`36`	`}`
`37`	`37`
`@@ -56,7 +56,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadExecute(`
`56`	`56`	`base_fd, std::initializer_list<Protection>{Protection::kRead,`
`57`	`57`	`Protection::kExecute});`
`58`	`58`
`59`		`- if (mapping->GetSize() == 0 \|\| mapping->GetMapping() == nullptr) {`
	`59`	`+ if (!mapping->IsValid()) {`
`60`	`60`	`return nullptr;`
`61`	`61`	`}`
`62`	`62`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,8 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,`
`63`	`63`	`return;`
`64`	`64`	`}`
`65`	`65`
`66`		`- if (stat_buffer.st_size <= 0) {`
	`66`	`+ if (stat_buffer.st_size == 0) {`
	`67`	`+ valid_ = true;`
`67`	`68`	`return;`
`68`	`69`	`}`
`69`	`70`
`@@ -79,6 +80,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,`
`79`	`80`
`80`	`81`	`mapping_ = static_cast<uint8_t*>(mapping);`
`81`	`82`	`size_ = stat_buffer.st_size;`
	`83`	`+ valid_ = true;`
`82`	`84`	`if (is_writable) {`
`83`	`85`	`mutable_mapping_ = mapping_;`
`84`	`86`	`}`
`@@ -98,4 +100,8 @@ const uint8_t* FileMapping::GetMapping() const {`
`98`	`100`	`return mapping_;`
`99`	`101`	`}`
`100`	`102`
	`103`	`+bool FileMapping::IsValid() const {`
	`104`	`+ return valid_;`
	`105`	`+}`
	`106`	`+`
`101`	`107`	`} // namespace fml`