commit 8ca234e6986763ffaddcf56b49fecdfe96a60182
author Kun Yi <kunyi731@gmail.com> Mon Mar 04 10:14:45 2019 -0800
committer Kun Yi <kunyi731@gmail.com> Fri Mar 08 10:45:08 2019 -0800
tree 426c4ab36e1e0f7ea3b4b0ae5d75ac6f865a00ed
parent 97be3afe2e85a52144f313b6df3b2940fbdec721

bugfix: Clean stale data on blob creation

It is possible that previous actions has stored stale data that are
valid protobufs, with different parameters. Detect stale data and
overwrite them.

Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I25b54885cdab928d1ad0b740d1c9b96335d2c60e

binarystore.cpp

diff --git a/binarystore.cpp b/binarystore.cpp
index 29e6b9f..a18a980 100644
--- a/binarystore.cpp
+++ b/binarystore.cpp
@@ -42,6 +42,11 @@
     auto store =
         std::make_unique<BinaryStore>(baseBlobId, std::move(file), maxSize);
 
+    if (!store->loadSerializedData())
+    {
+        return nullptr;
+    }
+
     return std::move(store);
 }
 
@@ -79,6 +84,19 @@
         return false;
     }
 
+    if (blob_.blob_base_id() != baseBlobId_)
+    {
+        /* Uh oh, stale data loaded. Clean it and commit. */
+        // TODO: it might be safer to add an option in config to error out
+        // instead of to overwrite.
+        log<level::ERR>("Stale blob data, resetting internals...",
+                        entry("LOADED=%s", blob_.blob_base_id().c_str()),
+                        entry("EXPECTED=%s", baseBlobId_.c_str()));
+        blob_.Clear();
+        blob_.set_blob_base_id(baseBlobId_);
+        return this->commit();
+    }
+
     return true;
 }
 

test/fake_sys_file.hpp

diff --git a/test/fake_sys_file.hpp b/test/fake_sys_file.hpp
index 1c6a45d..eb8d024 100644
--- a/test/fake_sys_file.hpp
+++ b/test/fake_sys_file.hpp
@@ -23,6 +23,13 @@
 class FakeSysFile : public SysFile
 {
   public:
+    FakeSysFile()
+    {
+    }
+    FakeSysFile(const std::string& s) : data_(s)
+    {
+    }
+
     size_t readToBuf(size_t pos, size_t count, char* buf) const override
     {
         auto result = readAsStr(pos, count);

test/handler_unittest.cpp

diff --git a/test/handler_unittest.cpp b/test/handler_unittest.cpp
index ef42ea6..31ccdc6 100644
--- a/test/handler_unittest.cpp
+++ b/test/handler_unittest.cpp
@@ -1,10 +1,18 @@
 #include "handler_unittest.hpp"
 
+#include "fake_sys_file.hpp"
+
+#include <google/protobuf/message_lite.h>
+#include <google/protobuf/text_format.h>
+
 #include <algorithm>
+#include <boost/endian/arithmetic.hpp>
 #include <memory>
 #include <string>
 #include <vector>
 
+#include "binaryblob.pb.h"
+
 using ::testing::_;
 using ::testing::AtLeast;
 using ::testing::ElementsAreArray;
@@ -140,4 +148,34 @@
     EXPECT_TRUE(handler.deleteBlob(basicTestBlobId));
 }
 
+TEST_F(BinaryStoreBlobHandlerBasicTest, StaleDataIsClearedDuringCreation)
+{
+    using namespace google::protobuf;
+
+    auto basicTestStaleBlobStr =
+        R"(blob_base_id: "/stale/"
+          blobs {
+            blob_id: "/stale/blob"
+          })";
+
+    // Create sysfile containing a valid but stale blob
+    const std::string staleBaseId = "/stale/"s;
+    const std::string staleBlobId = "/stale/blob"s;
+    binaryblobproto::BinaryBlobBase staleBlob;
+    EXPECT_TRUE(TextFormat::ParseFromString(basicTestStaleBlobStr, &staleBlob));
+
+    // Serialize to string stored in the fakeSysFile
+    auto staleBlobData = staleBlob.SerializeAsString();
+    boost::endian::little_uint64_t sizeLE = staleBlobData.size();
+    std::string commitData(sizeLE.data(), sizeof(sizeLE));
+    commitData += staleBlobData;
+
+    std::vector<std::string> expectedIdList = {basicTestBaseId};
+
+    handler.addNewBinaryStore(BinaryStore::createFromConfig(
+        basicTestBaseId, std::make_unique<FakeSysFile>(commitData), 0));
+    EXPECT_FALSE(handler.canHandleBlob(staleBlobId));
+    EXPECT_EQ(handler.getBlobIds(), expectedIdList);
+}
+
 } // namespace blobs