[arrow] branch master updated: ARROW-2784: [C++] MemoryMappedFile::WriteAt allow writing past the end

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 735e38b  ARROW-2784: [C++] MemoryMappedFile::WriteAt allow writing past the end
735e38b is described below

commit 735e38b2b906dfde32d4d294a9a606dab0566f88
Author: Dimitri Vorona <vo...@in.tum.de>
AuthorDate: Sun Jul 8 13:16:35 2018 -0400

    ARROW-2784: [C++] MemoryMappedFile::WriteAt allow writing past the end
    
    Author: Dimitri Vorona <vo...@in.tum.de>
    
    Closes #2206 from alendit/fix_write_past_the_end and squashes the following commits:
    
    a8987bb2 <Dimitri Vorona> Remove WriteAt position tests
    9fad80ce <Dimitri Vorona> Revert "Make WriteAt not move files position"
    35b40be0 <Dimitri Vorona> Make WriteAt not move files position
    c905183d <Dimitri Vorona> Check position before seeking and add more tests for ReadAt
    f0ed44d1 <Dimitri Vorona> Fix write beyond the end in MemoryMappedFile::ReatAt
---
 cpp/src/arrow/io/file.cc         |  4 +++
 cpp/src/arrow/io/io-file-test.cc | 55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/cpp/src/arrow/io/file.cc b/cpp/src/arrow/io/file.cc
index 8113482..19f4a35 100644
--- a/cpp/src/arrow/io/file.cc
+++ b/cpp/src/arrow/io/file.cc
@@ -515,6 +515,9 @@ Status MemoryMappedFile::WriteAt(int64_t position, const void* data, int64_t nby
   if (!memory_map_->opened() || !memory_map_->writable()) {
     return Status::IOError("Unable to write");
   }
+  if (position + nbytes > memory_map_->size()) {
+    return Status::Invalid("Cannot write past end of memory map");
+  }
 
   RETURN_NOT_OK(memory_map_->Seek(position));
   return WriteInternal(data, nbytes);
@@ -529,6 +532,7 @@ Status MemoryMappedFile::Write(const void* data, int64_t nbytes) {
   if (nbytes + memory_map_->position() > memory_map_->size()) {
     return Status::Invalid("Cannot write past end of memory map");
   }
+
   return WriteInternal(data, nbytes);
 }
 
diff --git a/cpp/src/arrow/io/io-file-test.cc b/cpp/src/arrow/io/io-file-test.cc
index 4d9120f..a06edca 100644
--- a/cpp/src/arrow/io/io-file-test.cc
+++ b/cpp/src/arrow/io/io-file-test.cc
@@ -614,6 +614,61 @@ TEST_F(TestMemoryMappedFile, WriteRead) {
   }
 }
 
+TEST_F(TestMemoryMappedFile, WriteAt) {
+  const int64_t buffer_size = 1024;
+  std::vector<uint8_t> buffer(buffer_size);
+  test::random_bytes(buffer_size, 0, buffer.data());
+
+  std::string path = "io-memory-map-write-read-test";
+  std::shared_ptr<MemoryMappedFile> result;
+  ASSERT_OK(InitMemoryMap(buffer_size, path, &result));
+
+  ASSERT_OK(result->WriteAt(0, buffer.data(), buffer_size / 2));
+
+  ASSERT_OK(
+      result->WriteAt(buffer_size / 2, buffer.data() + buffer_size / 2, buffer_size / 2));
+
+  std::shared_ptr<Buffer> out_buffer;
+  ASSERT_OK(result->ReadAt(0, buffer_size, &out_buffer));
+
+  ASSERT_EQ(memcmp(out_buffer->data(), buffer.data(), buffer_size), 0);
+}
+
+TEST_F(TestMemoryMappedFile, WriteBeyondEnd) {
+  const int64_t buffer_size = 1024;
+  std::vector<uint8_t> buffer(buffer_size);
+  test::random_bytes(buffer_size, 0, buffer.data());
+
+  std::string path = "io-memory-map-write-read-test";
+  std::shared_ptr<MemoryMappedFile> result;
+  ASSERT_OK(InitMemoryMap(buffer_size, path, &result));
+
+  ASSERT_OK(result->Seek(1));
+  ASSERT_RAISES(Invalid, result->Write(buffer.data(), buffer_size));
+
+  // The position should remain unchanged afterwards
+  int64_t position;
+  ASSERT_OK(result->Tell(&position));
+  ASSERT_EQ(position, 1);
+}
+
+TEST_F(TestMemoryMappedFile, WriteAtBeyondEnd) {
+  const int64_t buffer_size = 1024;
+  std::vector<uint8_t> buffer(buffer_size);
+  test::random_bytes(buffer_size, 0, buffer.data());
+
+  std::string path = "io-memory-map-write-read-test";
+  std::shared_ptr<MemoryMappedFile> result;
+  ASSERT_OK(InitMemoryMap(buffer_size, path, &result));
+
+  ASSERT_RAISES(Invalid, result->WriteAt(1, buffer.data(), buffer_size));
+
+  // The position should remain unchanged afterwards
+  int64_t position;
+  ASSERT_OK(result->Tell(&position));
+  ASSERT_EQ(position, 0);
+}
+
 TEST_F(TestMemoryMappedFile, GetSize) {
   std::string path = "io-memory-map-get-size";
   std::shared_ptr<MemoryMappedFile> result;