binarystore: Enable maxBinarySize Feature
Fail on write() and commit() if the result will exceed the max size.
Enabled by adding the max_binary_size to the proto buffer. The new one
takes priority and will replace existing one even if it already exists.
The `max_binary_size` is the size of the total data including the size
header. It is calculated with
```
blob_.SerializeAsString().size() +
sizeof(boost::endian::little_uint64_t)
```
Change-Id: I28a4c7a25fa066c11510b51cdfce27df4e09d3b5
Signed-off-by: Willy Tu <wltu@google.com>
diff --git a/binarystore.cpp b/binarystore.cpp
index e1cd03d..0760f33 100644
--- a/binarystore.cpp
+++ b/binarystore.cpp
@@ -11,6 +11,7 @@
#include <cstdint>
#include <ipmid/handler.hpp>
#include <memory>
+#include <optional>
#include <phosphor-logging/elog.hpp>
#include <string>
#include <vector>
@@ -30,7 +31,8 @@
std::unique_ptr<BinaryStoreInterface>
BinaryStore::createFromConfig(const std::string& baseBlobId,
- std::unique_ptr<SysFile> file)
+ std::unique_ptr<SysFile> file,
+ std::optional<uint32_t> maxSize)
{
if (baseBlobId.empty() || !file)
{
@@ -39,7 +41,8 @@
return nullptr;
}
- auto store = std::make_unique<BinaryStore>(baseBlobId, std::move(file));
+ auto store =
+ std::make_unique<BinaryStore>(baseBlobId, std::move(file), maxSize);
if (!store->loadSerializedData())
{
@@ -50,7 +53,8 @@
}
std::unique_ptr<BinaryStoreInterface>
- BinaryStore::createFromFile(std::unique_ptr<SysFile> file, bool readOnly)
+ BinaryStore::createFromFile(std::unique_ptr<SysFile> file, bool readOnly,
+ std::optional<uint32_t> maxSize)
{
if (!file)
{
@@ -58,7 +62,8 @@
return nullptr;
}
- auto store = std::make_unique<BinaryStore>(std::move(file), readOnly);
+ auto store =
+ std::make_unique<BinaryStore>(std::move(file), readOnly, maxSize);
if (!store->loadSerializedData())
{
@@ -92,6 +97,16 @@
* and is a valid case to handle. Simply init an empty binstore. */
commitState_ = CommitState::Uninitialized;
}
+
+ // The new max size takes priority
+ if (maxSize)
+ {
+ blob_.set_max_size_bytes(*maxSize);
+ }
+ else
+ {
+ blob_.clear_max_size_bytes();
+ }
}
catch (const std::system_error& e)
{
@@ -288,9 +303,25 @@
return false;
}
+ bool needResize = offset + data.size() > dataPtr->size();
+
+ // current size is the binary blob proto size + uint64 tracking the total
+ // size of the binary blob.
+ // currentSize = blob_size + x (uint64_t), where x = blob_size.
+ size_t currentSize = blob_.SerializeAsString().size() +
+ sizeof(boost::endian::little_uint64_t);
+ size_t sizeDelta = needResize ? offset + data.size() - dataPtr->size() : 0;
+
+ if (maxSize && currentSize + sizeDelta > *maxSize)
+ {
+ log<level::ERR>("Write data would make the total size exceed the max "
+ "size allowed. Return.");
+ return false;
+ }
+
commitState_ = CommitState::Dirty;
/* Copy (overwrite) the data */
- if (offset + data.size() > dataPtr->size())
+ if (needResize)
{
dataPtr->resize(offset + data.size()); // not enough space, extend
}
@@ -313,6 +344,13 @@
sizeof(sizeLE));
commitData += blobData;
+ // This should never be true if it is blocked by the write command
+ if (maxSize && sizeof(commitData) > *maxSize)
+ {
+ log<level::ERR>("Commit Data excedded maximum allowed size");
+ return false;
+ }
+
try
{
file_->writeStr(commitData, 0);
diff --git a/binarystore.hpp b/binarystore.hpp
index f46f69c..792de4a 100644
--- a/binarystore.hpp
+++ b/binarystore.hpp
@@ -8,6 +8,7 @@
#include <blobs-ipmid/blobs.hpp>
#include <cstdint>
#include <memory>
+#include <optional>
#include <string>
#include <vector>
@@ -42,15 +43,27 @@
};
BinaryStore() = delete;
- BinaryStore(const std::string& baseBlobId, std::unique_ptr<SysFile> file) :
- baseBlobId_(baseBlobId), file_(std::move(file))
+ BinaryStore(const std::string& baseBlobId, std::unique_ptr<SysFile> file,
+ std::optional<uint32_t> maxSize = std::nullopt) :
+ baseBlobId_(baseBlobId),
+ file_(std::move(file)), maxSize(maxSize)
{
blob_.set_blob_base_id(baseBlobId_);
+ if (maxSize)
+ {
+ blob_.set_max_size_bytes(*maxSize);
+ }
}
- BinaryStore(std::unique_ptr<SysFile> file, bool readOnly = false) :
- readOnly_{readOnly}, file_(std::move(file))
+ BinaryStore(std::unique_ptr<SysFile> file, bool readOnly = false,
+ std::optional<uint32_t> maxSize = std::nullopt) :
+ readOnly_{readOnly},
+ file_(std::move(file)), maxSize(maxSize)
{
+ if (maxSize)
+ {
+ blob_.set_max_size_bytes(*maxSize);
+ }
}
~BinaryStore() = default;
@@ -80,7 +93,8 @@
*/
static std::unique_ptr<BinaryStoreInterface>
createFromConfig(const std::string& baseBlobId,
- std::unique_ptr<SysFile> file);
+ std::unique_ptr<SysFile> file,
+ std::optional<uint32_t> maxSize = std::nullopt);
/**
* Helper factory method to create a BinaryStore instance
@@ -91,7 +105,8 @@
* @returns unique_ptr to constructed BinaryStore.
*/
static std::unique_ptr<BinaryStoreInterface>
- createFromFile(std::unique_ptr<SysFile> file, bool readOnly = true);
+ createFromFile(std::unique_ptr<SysFile> file, bool readOnly = true,
+ std::optional<uint32_t> maxSize = std::nullopt);
private:
/* Load the serialized data from sysfile if commit state is dirty.
@@ -107,6 +122,7 @@
bool readOnly_ = false;
std::unique_ptr<SysFile> file_ = nullptr;
CommitState commitState_ = CommitState::Dirty;
+ std::optional<uint32_t> maxSize;
};
} // namespace binstore
diff --git a/binarystore_mock.hpp b/binarystore_mock.hpp
index fa6c787..9360dd3 100644
--- a/binarystore_mock.hpp
+++ b/binarystore_mock.hpp
@@ -5,6 +5,7 @@
#include "sys_file.hpp"
#include <memory>
+#include <optional>
#include <string>
#include <vector>
@@ -19,8 +20,9 @@
{
public:
MockBinaryStore(const std::string& baseBlobId,
- std::unique_ptr<SysFile> file) :
- real_store_(baseBlobId, std::move(file))
+ std::unique_ptr<SysFile> file,
+ std::optional<uint32_t> maxSize = std::nullopt) :
+ real_store_(baseBlobId, std::move(file), maxSize)
{
// Implemented calls in BinaryStore will be directed to the real object.
ON_CALL(*this, getBaseBlobId)
diff --git a/main.cpp b/main.cpp
index 7c39eaf..5cf6c10 100644
--- a/main.cpp
+++ b/main.cpp
@@ -71,7 +71,7 @@
config.offsetBytes);
handler->addNewBinaryStore(binstore::BinaryStore::createFromConfig(
- config.blobBaseId, std::move(file)));
+ config.blobBaseId, std::move(file), config.maxSizeBytes));
}
return handler;
diff --git a/test/binarystore_unittest.cpp b/test/binarystore_unittest.cpp
index f0e8048..2a00696 100644
--- a/test/binarystore_unittest.cpp
+++ b/test/binarystore_unittest.cpp
@@ -9,6 +9,7 @@
#include <iterator>
#include <memory>
#include <sstream>
+#include <vector>
#include <gmock/gmock.h>
@@ -37,6 +38,11 @@
"\""
"}] "
"max_size_bytes: 64";
+const std::string smallInputProto = "blob_base_id: \"/s/test\""
+ "blobs: [{ "
+ " blob_id: \"/s/test/0\""
+ "}] "
+ "max_size_bytes: 64";
class SysFileBuf : public binstore::SysFile
{
@@ -167,3 +173,44 @@
EXPECT_FALSE(store->openOrCreateBlob(
"/blob/my-test/2", blobs::OpenFlags::read & blobs::OpenFlags::write));
}
+
+TEST_F(BinaryStoreTest, TestWriteExceedMaxSize)
+{
+ std::vector<uint8_t> writeData(10, 0);
+ auto testDataFile = createBlobStorage(smallInputProto);
+ auto store = binstore::BinaryStore::createFromConfig(
+ "/s/test", std::move(testDataFile), 48);
+ ASSERT_TRUE(store);
+
+ EXPECT_TRUE(store->openOrCreateBlob(
+ "/s/test/0", blobs::OpenFlags::write | blobs::OpenFlags::read));
+ // Current size 24(blob_ + max_size) + 8(size var) = 32
+ EXPECT_TRUE(store->write(
+ 0, writeData)); // 44 = 32(existing) + 10 (data) + 2 (blob_id '/0')
+ EXPECT_FALSE(
+ store->write(10, writeData)); // 54 = 44 (existing) + 10 (new data)
+ EXPECT_FALSE(
+ store->write(5, writeData)); // 49 = 44 (existing) + 5 (new data)
+ EXPECT_TRUE(
+ store->write(4, writeData)); // 48 = 44 (existing) + 4 (new data)
+}
+
+TEST_F(BinaryStoreTest, TestCreateFromConfigExceedMaxSize)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto store = binstore::BinaryStore::createFromConfig(
+ "/blob/my-test", std::move(testDataFile), 1);
+ ASSERT_TRUE(store);
+ EXPECT_FALSE(store->commit());
+}
+
+TEST_F(BinaryStoreTest, TestCreateFromFileExceedMaxSize)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto store = binstore::BinaryStore::createFromFile(std::move(testDataFile),
+ false, 1);
+
+ // Reading from File is expected to call loadSerializedData and fail at the
+ // commit()
+ EXPECT_FALSE(store);
+}