CL16570338: CompactBinary: Added validation to LoadCompactBinary and removed asserts from the other load functions

1 files changed, 68 insertions, 34 deletions

diff --git a/zencore/compactbinarypackage.cpp b/zencore/compactbinarypackage.cpp
index 4a1ea65d2..b0dbd2918 100644
--- a/zencore/compactbinarypackage.cpp
+++ b/zencore/compactbinarypackage.cpp
@@ -97,27 +97,34 @@ CbAttachment::AsCompactBinary() const
 	return CompactBinary ? CbFieldIterator::MakeRangeView(CompactBinary, Buffer) : CbFieldIterator();
 }
 
-void
-CbAttachment::Load(IoBuffer& InBuffer, BufferAllocator Allocator)
+bool
+CbAttachment::TryLoad(IoBuffer& InBuffer, BufferAllocator Allocator)
 {
 	MemoryInStream InStream(InBuffer.Data(), InBuffer.Size());
 	BinaryReader   Reader(InStream);
 
-	Load(Reader, Allocator);
+	return TryLoad(Reader, Allocator);
 }
 
-void
-CbAttachment::Load(CbFieldIterator& Fields)
+bool
+CbAttachment::TryLoad(CbFieldIterator& Fields)
 {
-	ZEN_ASSERT(Fields.IsBinary());	//, TEXT("Attachments must start with a binary field."));
 	const MemoryView View = Fields.AsBinaryView();
+	if (Fields.HasError())
+	{
+		return false;
+	}
+
 	if (View.GetSize() > 0)
 	{
 		Buffer = SharedBuffer::MakeView(View, Fields.GetOuterBuffer());
 		Buffer.MakeOwned();
 		++Fields;
 		Hash = Fields.AsAttachment();
-		ZEN_ASSERT(!Fields.HasError());	 // TEXT("Attachments must be a non-empty binary value with a content hash."));
+		if (Fields.HasError())
+		{
+			return false;
+		}
 		if (Fields.IsObjectAttachment())
 		{
 			CompactBinary = CbFieldViewIterator::MakeRange(Buffer);
@@ -131,14 +138,19 @@ CbAttachment::Load(CbFieldIterator& Fields)
 		CompactBinary.Reset();
 		Hash = IoHash::Zero;
 	}
+
+	return true;
 }
 
-void
-CbAttachment::Load(BinaryReader& Reader, BufferAllocator Allocator)
+bool
+CbAttachment::TryLoad(BinaryReader& Reader, BufferAllocator Allocator)
 {
-	CbField BufferField = LoadCompactBinary(Reader, Allocator);
-	ZEN_ASSERT(BufferField.IsBinary(), "Attachments must start with a binary field");
-	const MemoryView View = BufferField.AsBinaryView();
+	CbField			 BufferField = LoadCompactBinary(Reader, Allocator);
+	const MemoryView View		 = BufferField.AsBinaryView();
+	if (BufferField.HasError())
+	{
+		return false;
+	}
 	if (View.GetSize() > 0)
 	{
 		Buffer = SharedBuffer::MakeView(View, BufferField.GetOuterBuffer());
@@ -151,7 +163,10 @@ CbAttachment::Load(BinaryReader& Reader, BufferAllocator Allocator)
 			 return UniqueBuffer::MakeMutableView(HashBuffer.data(), Size);
 		 });
 		Hash						   = HashField.AsAttachment();
-		ZEN_ASSERT(!HashField.HasError(), "Attachments must be a non-empty binary value with a content hash.");
+		if (HashField.HasError() || IoHash::HashMemory(Buffer) != Hash)
+		{
+			return false;
+		}
 		if (HashField.IsObjectAttachment())
 		{
 			CompactBinary = CbFieldViewIterator::MakeRange(Buffer);
@@ -163,6 +178,7 @@ CbAttachment::Load(BinaryReader& Reader, BufferAllocator Allocator)
 		CompactBinary.Reset();
 		Hash = IoHash::Zero;
 	}
+	return true;
 }
 
 void
@@ -301,17 +317,17 @@ CbPackage::GatherAttachments(const CbFieldViewIterator& Fields, AttachmentResolv
 	});
 }
 
-void
-CbPackage::Load(IoBuffer& InBuffer, BufferAllocator Allocator, AttachmentResolver* Mapper)
+bool
+CbPackage::TryLoad(IoBuffer& InBuffer, BufferAllocator Allocator, AttachmentResolver* Mapper)
 {
 	MemoryInStream InStream(InBuffer.Data(), InBuffer.Size());
 	BinaryReader   Reader(InStream);
 
-	Load(Reader, Allocator, Mapper);
+	return TryLoad(Reader, Allocator, Mapper);
 }
 
-void
-CbPackage::Load(CbFieldIterator& Fields)
+bool
+CbPackage::TryLoad(CbFieldIterator& Fields)
 {
 	*this = CbPackage();
 	while (Fields)
@@ -324,19 +340,25 @@ CbPackage::Load(CbFieldIterator& Fields)
 		else if (Fields.IsBinary())
 		{
 			CbAttachment Attachment;
-			Attachment.Load(Fields);
+			Attachment.TryLoad(Fields);
 			AddAttachment(Attachment);
 		}
 		else
 		{
-			ZEN_ASSERT(Fields.IsObject(), TEXT("Expected Object, Binary, or Null field when loading a package."));
 			Object = Fields.AsObject();
+			if (Fields->HasError())
+			{
+				return false;
+			}
 			Object.MakeOwned();
 			++Fields;
 			if (Object.CreateIterator())
 			{
 				ObjectHash = Fields.AsObjectAttachment();
-				ZEN_ASSERT(!Fields.HasError(), TEXT("Object must be followed by a CompactBinaryReference with the object hash."));
+				if (Fields.HasError())
+				{
+					return false;
+				}
 				++Fields;
 			}
 			else
@@ -345,10 +367,12 @@ CbPackage::Load(CbFieldIterator& Fields)
 			}
 		}
 	}
+
+	return true;
 }
 
-void
-CbPackage::Load(BinaryReader& Reader, BufferAllocator Allocator, AttachmentResolver* Mapper)
+bool
+CbPackage::TryLoad(BinaryReader& Reader, BufferAllocator Allocator, AttachmentResolver* Mapper)
 {
 	uint8_t	   StackBuffer[64];
 	const auto StackAllocator = [&Allocator, &StackBuffer](uint64_t Size) -> UniqueBuffer {
@@ -365,9 +389,13 @@ CbPackage::Load(BinaryReader& Reader, BufferAllocator Allocator, AttachmentResol
 	for (;;)
 	{
 		CbField ValueField = LoadCompactBinary(Reader, StackAllocator);
+		if (!ValueField)
+		{
+			return false;
+		}
 		if (ValueField.IsNull())
 		{
-			break;
+			return true;
 		}
 		else if (ValueField.IsBinary())
 		{
@@ -399,14 +427,20 @@ CbPackage::Load(BinaryReader& Reader, BufferAllocator Allocator, AttachmentResol
 		}
 		else
 		{
-			ZEN_ASSERT(ValueField.IsObject(), "Expected Object, Binary, or Null field when loading a package");
 			Object = ValueField.AsObject();
+			if (ValueField.HasError())
+			{
+				return false;
+			}
 			Object.MakeOwned();
 			if (Object.CreateViewIterator())
 			{
 				CbField HashField = LoadCompactBinary(Reader, StackAllocator);
 				ObjectHash		  = HashField.AsObjectAttachment();
-				ZEN_ASSERT(!HashField.HasError(), "Object must be followed by a ObjectAttachment with the object hash.");
+				if (HashField.HasError() || Object.GetHash() != ObjectHash)
+				{
+					return false;
+				}
 			}
 			else
 			{
@@ -466,14 +500,14 @@ TEST_CASE("usonpackage")
 		CHECK(ValidateObjectAttachment(MakeMemoryView(WriteStream), CbValidateMode::All) == CbValidateError::None);
 
 		CbAttachment FromFields;
-		FromFields.Load(Fields);
+		FromFields.TryLoad(Fields);
 		CHECK(!bool(Fields));
 		CHECK(FromFields == Attachment);
 
 		CbAttachment   FromArchive;
 		MemoryInStream InStream(MakeMemoryView(WriteStream));
 		BinaryReader   Reader(InStream);
-		FromArchive.Load(Reader);
+		FromArchive.TryLoad(Reader);
 		CHECK(Reader.CurrentOffset() == InStream.Size());
 		CHECK(FromArchive == Attachment);
 	};
@@ -564,7 +598,7 @@ TEST_CASE("usonpackage")
 		Attachment.Save(Writer);
 		CbFieldIterator Fields	   = Writer.Save();
 		CbFieldIterator FieldsView = CbFieldIterator::MakeRangeView(CbFieldViewIterator(Fields));
-		Attachment.Load(FieldsView);
+		Attachment.TryLoad(FieldsView);
 
 		CHECK_FALSE(Attachment.IsNull());
 		CHECK(bool(Attachment));
@@ -590,7 +624,7 @@ TEST_CASE("usonpackage")
 		CbFieldIterator Fields	   = Writer.Save();
 		CbFieldIterator FieldsView = CbFieldIterator::MakeRangeView(CbFieldViewIterator(Fields));
 
-		Attachment.Load(FieldsView);
+		Attachment.TryLoad(FieldsView);
 
 		CHECK_FALSE(Attachment.IsNull());
 		CHECK(bool(Attachment));
@@ -669,14 +703,14 @@ TEST_CASE("usonpackage.serialization")
 		CHECK(ValidateCompactBinaryPackage(MakeMemoryView(MemStream), CbValidateMode::All) == CbValidateError::None);
 
 		CbPackage FromFields;
-		FromFields.Load(Fields);
+		FromFields.TryLoad(Fields);
 		CHECK_FALSE(bool(Fields));
 		CHECK(FromFields == Package);
 
 		CbPackage	   FromArchive;
 		MemoryInStream ReadMemStream(MakeMemoryView(MemStream));
 		BinaryReader   ReadAr(ReadMemStream);
-		FromArchive.Load(ReadAr);
+		FromArchive.TryLoad(ReadAr);
 		CHECK(ReadAr.CurrentOffset() == ReadMemStream.Size());
 		CHECK(FromArchive == Package);
 	};
@@ -869,7 +903,7 @@ TEST_CASE("usonpackage.serialization")
 
 		CbFieldIterator Fields = Writer.Save();
 		CbPackage		FromFields;
-		FromFields.Load(Fields);
+		FromFields.TryLoad(Fields);
 
 		const CbAttachment* const Level2Attachment = FromFields.FindAttachment(Level2Hash);
 		REQUIRE(Level2Attachment);
@@ -902,7 +936,7 @@ TEST_CASE("usonpackage.serialization")
 		CbPackage	   FromArchive;
 		MemoryInStream ReadStream(MakeMemoryView(WriteStream));
 		BinaryReader   ReadAr(ReadStream);
-		FromArchive.Load(ReadAr);
+		FromArchive.TryLoad(ReadAr);
 
 		Writer.Reset();
 		FromArchive.Save(Writer);