fix zero size attachment replies (#69)

- Bugfix: Don't try to respond with zero size partial cache value when partial size is zero
- Improvement: Added more validation of data read from cache / cas

2 files changed, 44 insertions, 62 deletions

diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp
index a4fa38a1d..4d4e0951e 100644
--- a/src/zencore/compactbinarypackage.cpp
+++ b/src/zencore/compactbinarypackage.cpp
@@ -11,32 +11,45 @@ namespace zen {
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-CbAttachment::CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash) : CbAttachment(InValue.MakeOwned(), Hash)
+CbAttachment::CbAttachment(const CbObject& InValue, const IoHash* const InHash)
 {
-}
+	auto SetValue = [&](const CbObject& ValueToSet) {
+		if (InHash)
+		{
+			Value.emplace<CbObject>(ValueToSet);
+			Hash = *InHash;
+		}
+		else
+		{
+			Value.emplace<CbObject>(ValueToSet);
+			Hash = ValueToSet.GetHash();
+		}
+	};
 
-CbAttachment::CbAttachment(const SharedBuffer& InValue) : CbAttachment(CompositeBuffer(InValue))
-{
+	MemoryView View;
+	if (!InValue.IsOwned() || !InValue.TryGetSerializedView(View))
+	{
+		SetValue(CbObject::Clone(InValue));
+	}
+	else
+	{
+		SetValue(InValue);
+	}
 }
 
-CbAttachment::CbAttachment(const SharedBuffer& InValue, const IoHash& InHash) : CbAttachment(CompositeBuffer(InValue), InHash)
+CbAttachment::CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash) : Hash(Hash), Value(InValue)
 {
+	ZEN_ASSERT(!std::get<CompressedBuffer>(Value).IsNull());
 }
 
-CbAttachment::CbAttachment(const CompositeBuffer& InValue)
-: Hash(InValue.IsNull() ? IoHash::Zero : IoHash::HashBuffer(InValue))
-, Value(InValue)
+CbAttachment::CbAttachment(CompressedBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(std::move(InValue))
 {
-	if (std::get<CompositeBuffer>(Value).IsNull())
-	{
-		Value.emplace<std::nullptr_t>();
-	}
+	ZEN_ASSERT(!std::get<CompressedBuffer>(Value).IsNull());
 }
 
 CbAttachment::CbAttachment(CompositeBuffer&& InValue)
 : Hash(InValue.IsNull() ? IoHash::Zero : IoHash::HashBuffer(InValue))
 , Value(std::move(InValue))
-
 {
 	if (std::get<CompositeBuffer>(Value).IsNull())
 	{
@@ -44,7 +57,7 @@ CbAttachment::CbAttachment(CompositeBuffer&& InValue)
 	}
 }
 
-CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(InValue)
+CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(std::move(InValue))
 {
 	if (std::get<CompositeBuffer>(Value).IsNull())
 	{
@@ -52,40 +65,6 @@ CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Ha
 	}
 }
 
-CbAttachment::CbAttachment(CompressedBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(InValue)
-{
-	if (std::get<CompressedBuffer>(Value).IsNull())
-	{
-		Value.emplace<std::nullptr_t>();
-	}
-}
-
-CbAttachment::CbAttachment(const CbObject& InValue, const IoHash* const InHash)
-{
-	auto SetValue = [&](const CbObject& ValueToSet) {
-		if (InHash)
-		{
-			Value.emplace<CbObject>(ValueToSet);
-			Hash = *InHash;
-		}
-		else
-		{
-			Value.emplace<CbObject>(ValueToSet);
-			Hash = ValueToSet.GetHash();
-		}
-	};
-
-	MemoryView View;
-	if (!InValue.IsOwned() || !InValue.TryGetSerializedView(View))
-	{
-		SetValue(CbObject::Clone(InValue));
-	}
-	else
-	{
-		SetValue(InValue);
-	}
-}
-
 bool
 CbAttachment::TryLoad(IoBuffer& InBuffer, BufferAllocator Allocator)
 {
@@ -186,7 +165,7 @@ TryLoad_ArchiveFieldIntoAttachment(CbAttachment& TargetAttachment, CbField&& Fie
 		{
 			return false;
 		}
-		TargetAttachment = CbAttachment(CompositeBuffer(Buffer), BinaryAttachmentHash);
+		TargetAttachment = CbAttachment(std::move(Buffer), BinaryAttachmentHash);
 	}
 	else if (SharedBuffer Buffer = Field.AsBinary(); !Field.HasError())
 	{
@@ -201,7 +180,7 @@ TryLoad_ArchiveFieldIntoAttachment(CbAttachment& TargetAttachment, CbField&& Fie
 		else
 		{
 			// Is an uncompressed empty binary blob
-			TargetAttachment = CbAttachment(CompositeBuffer(Buffer), IoHash::HashBuffer(nullptr, 0));
+			TargetAttachment = CbAttachment(std::move(Buffer), IoHash::HashBuffer(nullptr, 0));
 		}
 	}
 	else
@@ -386,6 +365,10 @@ CbPackage::AddAttachments(std::span<const CbAttachment> InAttachments)
 	{
 		return;
 	}
+	for (const CbAttachment& Attachment : InAttachments)
+	{
+		ZEN_ASSERT(!Attachment.IsNull());
+	}
 	// Assume we have no duplicates!
 	Attachments.insert(Attachments.end(), InAttachments.begin(), InAttachments.end());
 	std::sort(Attachments.begin(), Attachments.end());
@@ -741,7 +724,7 @@ namespace legacy {
 						}
 						else
 						{
-							Package.AddAttachment(CbAttachment(CompositeBuffer(std::move(Buffer)), Hash));
+							Package.AddAttachment(CbAttachment(std::move(Buffer), Hash));
 						}
 					}
 				}
diff --git a/src/zencore/include/zencore/compactbinarypackage.h b/src/zencore/include/zencore/compactbinarypackage.h
index 16f723edc..4a6bca67a 100644
--- a/src/zencore/include/zencore/compactbinarypackage.h
+++ b/src/zencore/include/zencore/compactbinarypackage.h
@@ -46,26 +46,23 @@ public:
 	inline explicit CbAttachment(const CbObject& InValue) : CbAttachment(InValue, nullptr) {}
 
 	/** Construct a compact binary attachment. Value is cloned if not owned. Hash must match Value. */
-	inline explicit CbAttachment(const CbObject& InValue, const IoHash& Hash) : CbAttachment(InValue, &Hash) {}
+	inline CbAttachment(const CbObject& InValue, const IoHash& Hash) : CbAttachment(InValue, &Hash) {}
 
 	/** Construct a raw binary attachment. Value is cloned if not owned. */
-	ZENCORE_API explicit CbAttachment(const SharedBuffer& InValue);
+	ZENCORE_API explicit CbAttachment(const SharedBuffer& InValue) : CbAttachment(CompositeBuffer(InValue)) {}
 
 	/** Construct a raw binary attachment. Value is cloned if not owned. Hash must match Value. */
-	ZENCORE_API explicit CbAttachment(const SharedBuffer& InValue, const IoHash& Hash);
+	ZENCORE_API CbAttachment(const SharedBuffer& InValue, const IoHash& Hash) : CbAttachment(CompositeBuffer(InValue), Hash) {}
 
 	/** Construct a raw binary attachment. Value is cloned if not owned. */
-	ZENCORE_API explicit CbAttachment(const CompositeBuffer& InValue);
+	ZENCORE_API explicit CbAttachment(SharedBuffer&& InValue) : CbAttachment(CompositeBuffer(std::move(InValue))) {}
 
-	/** Construct a raw binary attachment. Value is cloned if not owned. */
-	ZENCORE_API explicit CbAttachment(CompositeBuffer&& InValue);
-
-	/** Construct a raw binary attachment. Value is cloned if not owned. */
-	ZENCORE_API explicit CbAttachment(CompositeBuffer&& InValue, const IoHash& Hash);
+	/** Construct a raw binary attachment. Value is cloned if not owned. Hash must match Value. */
+	ZENCORE_API CbAttachment(SharedBuffer&& InValue, const IoHash& Hash) : CbAttachment(CompositeBuffer(std::move(InValue)), Hash) {}
 
 	/** Construct a compressed binary attachment. Value is cloned if not owned. */
-	ZENCORE_API explicit CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash);
-	ZENCORE_API explicit CbAttachment(CompressedBuffer&& InValue, const IoHash& Hash);
+	ZENCORE_API CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash);
+	ZENCORE_API CbAttachment(CompressedBuffer&& InValue, const IoHash& Hash);
 
 	/** Reset this to a null attachment. */
 	inline void Reset() { *this = CbAttachment(); }
@@ -132,6 +129,8 @@ public:
 
 private:
 	ZENCORE_API CbAttachment(const CbObject& Value, const IoHash* Hash);
+	ZENCORE_API explicit CbAttachment(CompositeBuffer&& InValue);
+	ZENCORE_API CbAttachment(CompositeBuffer&& InValue, const IoHash& Hash);
 
 	IoHash																	  Hash;
 	std::variant<std::nullptr_t, CbObject, CompositeBuffer, CompressedBuffer> Value;