From bcc85c7e5cbcf0737f5180e3d601e63b480e8949 Mon Sep 17 00:00:00 2001
From: Dan Engelbrecht <dan.engelbrecht@epicgames.com>
Date: Fri, 30 Sep 2022 15:13:45 +0200
Subject: De/reduce buffer creation in parsepackedmessage (#175)

* Don't create call CreateBuffer for attachement data that we only read and not keep
* changelog
* don't read oplog attachments into memory just to do a redundant store of them
---
 zenhttp/httpshared.cpp | 49 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 12 deletions(-)

(limited to 'zenhttp/httpshared.cpp')

diff --git a/zenhttp/httpshared.cpp b/zenhttp/httpshared.cpp
index 2b96f32fc..f9aa3af82 100644
--- a/zenhttp/httpshared.cpp
+++ b/zenhttp/httpshared.cpp
@@ -253,14 +253,11 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 
 	for (uint32_t i = 0; i < ChunkCount; ++i)
 	{
-		const CbAttachmentEntry& Entry			  = AttachmentEntries[i];
-		const uint64_t			 AttachmentSize	  = Entry.PayloadSize;
-		IoBuffer				 AttachmentBuffer = CreateBuffer(Entry.AttachmentHash, AttachmentSize);
+		const CbAttachmentEntry& Entry			= AttachmentEntries[i];
+		const uint64_t			 AttachmentSize = Entry.PayloadSize;
 
-		ZEN_ASSERT(AttachmentBuffer);
-		ZEN_ASSERT(AttachmentBuffer.Size() == AttachmentSize);
-
-		Reader.Read(AttachmentBuffer.MutableData(), AttachmentSize);
+		const IoBuffer AttachmentBuffer(Payload, Reader.CurrentOffset(), AttachmentSize);
+		Reader.Skip(AttachmentSize);
 
 		if (Entry.Flags & CbAttachmentEntry::kIsLocalRef)
 		{
@@ -279,13 +276,20 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 					IoBufferBuilder::MakeFromFile(Path, AttachRefHdr->PayloadByteOffset, AttachRefHdr->PayloadByteSize))
 			{
 				CompressedBuffer CompBuf(CompressedBuffer::FromCompressed(SharedBuffer(ChunkReference)));
-				CbAttachment	 Attachment(std::move(CompBuf));
+				if (!CompBuf)
+				{
+					throw std::runtime_error(fmt::format("invalid format for chunk #{} at '{}' (offset {}, size {})",
+														 i,
+														 PathToUtf8(Path),
+														 AttachRefHdr->PayloadByteOffset,
+														 AttachRefHdr->PayloadByteSize));
+				}
+				CbAttachment Attachment(std::move(CompBuf));
 				Package.AddAttachment(Attachment);
 			}
 			else
 			{
 				// Unable to open chunk reference
-
 				throw std::runtime_error(fmt::format("unable to resolve chunk #{} at '{}' (offset {}, size {})",
 													 i,
 													 PathToUtf8(Path),
@@ -295,12 +299,15 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 		}
 		else if (Entry.Flags & CbAttachmentEntry::kIsCompressed)
 		{
-			CompressedBuffer CompBuf(CompressedBuffer::FromCompressed(SharedBuffer(AttachmentBuffer)));
-
 			if (Entry.Flags & CbAttachmentEntry::kIsObject)
 			{
 				if (i == 0)
 				{
+					CompressedBuffer CompBuf(CompressedBuffer::FromCompressed(SharedBuffer(AttachmentBuffer)));
+					if (!CompBuf)
+					{
+						throw std::runtime_error(fmt::format("invalid format for chunk #{} expected compressed buffer for CbObject", i));
+					}
 					// First payload is always a compact binary object
 					Package.SetObject(LoadCompactBinaryObject(std::move(CompBuf)));
 				}
@@ -311,6 +318,18 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 			}
 			else
 			{
+				// Make a copy of the buffer so we attachements don't reference the entire payload
+				IoBuffer AttachmentBufferCopy = CreateBuffer(Entry.AttachmentHash, AttachmentSize);
+				ZEN_ASSERT(AttachmentBufferCopy);
+				ZEN_ASSERT(AttachmentBufferCopy.Size() == AttachmentSize);
+				AttachmentBufferCopy.GetMutableView().CopyFrom(AttachmentBuffer.GetView());
+
+				CompressedBuffer CompBuf(CompressedBuffer::FromCompressed(SharedBuffer(AttachmentBufferCopy)));
+				if (!CompBuf)
+				{
+					throw std::runtime_error(fmt::format("invalid format for chunk #{} expected compressed buffer for attachment", i));
+				}
+
 				CbAttachment Attachment(std::move(CompBuf));
 				Package.AddAttachment(Attachment);
 			}
@@ -330,7 +349,13 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 			}
 			else
 			{
-				CbAttachment Attachment(SharedBuffer{AttachmentBuffer});
+				// Make a copy of the buffer so we attachements don't reference the entire payload
+				IoBuffer AttachmentBufferCopy = CreateBuffer(Entry.AttachmentHash, AttachmentSize);
+				ZEN_ASSERT(AttachmentBufferCopy);
+				ZEN_ASSERT(AttachmentBufferCopy.Size() == AttachmentSize);
+				AttachmentBufferCopy.GetMutableView().CopyFrom(AttachmentBuffer.GetView());
+
+				CbAttachment Attachment(SharedBuffer{AttachmentBufferCopy});
 				Package.AddAttachment(Attachment);
 			}
 		}
-- 
cgit v1.2.3