From 1155ed2048a24f4dfcfa65d63243b77973f820d6 Mon Sep 17 00:00:00 2001
From: Dan Engelbrecht <dan.engelbrecht@epicgames.com>
Date: Thu, 2 May 2024 17:40:30 +0200
Subject: 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
---
 src/zencore/compactbinarypackage.cpp | 85 +++++++++++++++---------------------
 1 file changed, 34 insertions(+), 51 deletions(-)

(limited to 'src/zencore/compactbinarypackage.cpp')

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));
 						}
 					}
 				}
-- 
cgit v1.2.3