aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md2
-rw-r--r--src/zencore/compactbinarypackage.cpp85
-rw-r--r--src/zencore/include/zencore/compactbinarypackage.h21
-rw-r--r--src/zenserver/cache/httpstructuredcache.cpp10
-rw-r--r--src/zenserver/projectstore/projectstore.cpp13
-rw-r--r--src/zenserver/projectstore/zenremoteprojectstore.cpp1
-rw-r--r--src/zenstore/cache/cacherpc.cpp7
-rw-r--r--src/zenutil/packageformat.cpp12
8 files changed, 84 insertions, 67 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index deb95a381..a1f2d44ad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,9 +1,11 @@
##
- Bugfix: Remove extra loop causing GetProjectFiles for project store to find all chunks once for each chunk found
- Bugfix: Don't capture ChunkIndex variable in CasImpl::IterateChunks by reference as it causes crash
+- Bugfix: Don't try to respond with zero size partial cache value when partial size is zero
- Improvement: Make FileCasStrategy::IterateChunks (optionally) multithreaded (improves GetProjectFiles performance)
- Improvement: Add batch scope for adding multiple cache values from single request efficently
- Improvement: Use temp file write and move into place for manifest/state files to avoid partial incomplete file writes
+- Improvement: Added more validation of data read from cache / cas
## 5.5.0
- Change: GCv2 is now the default option, use `--gc-v2=false` to fall back to GCv1
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;
diff --git a/src/zenserver/cache/httpstructuredcache.cpp b/src/zenserver/cache/httpstructuredcache.cpp
index 135eee57c..449a43653 100644
--- a/src/zenserver/cache/httpstructuredcache.cpp
+++ b/src/zenserver/cache/httpstructuredcache.cpp
@@ -727,7 +727,15 @@ HttpStructuredCacheService::HandleGetCacheRecord(HttpServerRequest& Request, con
if (IoBuffer Chunk = m_CidStore.FindChunkByCid(AttachmentHash.AsHash()))
{
CompressedBuffer Compressed = CompressedBuffer::FromCompressedNoValidate(std::move(Chunk));
- Package.AddAttachment(CbAttachment(Compressed, AttachmentHash.AsHash()));
+ if (Compressed)
+ {
+ Package.AddAttachment(CbAttachment(Compressed, AttachmentHash.AsHash()));
+ }
+ else
+ {
+ ZEN_WARN("invalid compressed binary returned for {}", AttachmentHash.AsHash());
+ MissingCount++;
+ }
}
else
{
diff --git a/src/zenserver/projectstore/projectstore.cpp b/src/zenserver/projectstore/projectstore.cpp
index f5fecce24..45a96c60d 100644
--- a/src/zenserver/projectstore/projectstore.cpp
+++ b/src/zenserver/projectstore/projectstore.cpp
@@ -3431,9 +3431,16 @@ ProjectStore::Rpc(HttpServerRequest& HttpReq,
IoBuffer ChunkBuffer = m_CidStore.FindChunkByCid(RawHash);
if (ChunkBuffer)
{
- ResponseWriter.AddHash(RawHash);
- ResponsePackage.AddAttachment(
- CbAttachment(CompressedBuffer::FromCompressedNoValidate(std::move(ChunkBuffer)), RawHash));
+ CompressedBuffer Compressed = CompressedBuffer::FromCompressedNoValidate(std::move(ChunkBuffer));
+ if (Compressed)
+ {
+ ResponseWriter.AddHash(RawHash);
+ ResponsePackage.AddAttachment(CbAttachment(std::move(Compressed), RawHash));
+ }
+ else
+ {
+ ZEN_WARN("invalid compressed binary in cas store for {}", RawHash);
+ }
}
}
ResponseWriter.EndArray();
diff --git a/src/zenserver/projectstore/zenremoteprojectstore.cpp b/src/zenserver/projectstore/zenremoteprojectstore.cpp
index cec68111f..679c344c6 100644
--- a/src/zenserver/projectstore/zenremoteprojectstore.cpp
+++ b/src/zenserver/projectstore/zenremoteprojectstore.cpp
@@ -119,6 +119,7 @@ public:
IoHash RawHash;
uint64_t RawSize;
CompressedBuffer Compressed = CompressedBuffer::FromCompressed(Chunk, RawHash, RawSize);
+ ZEN_ASSERT(Compressed);
RequestWriter.AddHash(RawHash);
RequestPackage.AddAttachment(CbAttachment(Compressed, RawHash));
}
diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp
index 6871dfd56..5bd73e902 100644
--- a/src/zenstore/cache/cacherpc.cpp
+++ b/src/zenstore/cache/cacherpc.cpp
@@ -1632,6 +1632,10 @@ CacheRpcHandler::WriteGetCacheChunksResponse([[maybe_unused]] const CacheRequest
if (Request.Value && !EnumHasAllFlags(Request.DownstreamPolicy, CachePolicy::SkipData))
{
auto IsPartialRangeRequest = [](const ChunkRequest& Request) {
+ if (Request.RequestedOffset == 0 && Request.RequestedSize == 0)
+ {
+ return false;
+ }
if ((Request.RequestedOffset != 0) || (Request.RequestedSize != ~uint64_t(0)))
{
return true;
@@ -1657,7 +1661,8 @@ CacheRpcHandler::WriteGetCacheChunksResponse([[maybe_unused]] const CacheRequest
FragmentRawOffset = Request.RequestedOffset;
}
}
- Request.Value = Request.Value.GetRange(Request.RequestedOffset, Request.RequestedSize);
+ Request.Value =
+ Request.Value.GetRange(Request.RequestedOffset, Request.RequestedSize == 0 ? 1u : Request.RequestedSize);
uint64_t FragmentLength = Request.Value.DecodeRawSize();
IoHashStream FragmentHashStream;
diff --git a/src/zenutil/packageformat.cpp b/src/zenutil/packageformat.cpp
index 2512351f5..963a442cd 100644
--- a/src/zenutil/packageformat.cpp
+++ b/src/zenutil/packageformat.cpp
@@ -51,6 +51,7 @@ FormatPackageMessageBuffer(const CbPackage& Data, FormatFlags Flags, void* Targe
for (IoBuffer& Buf : Message)
{
+ ZEN_ASSERT(Buf.GetSize() > 0);
Buffers.push_back(SharedBuffer(Buf));
}
@@ -200,6 +201,7 @@ FormatPackageMessage(const CbPackage& Data, FormatFlags Flags, void* TargetProce
// Root object
IoBuffer RootIoBuffer = Data.GetObject().GetBuffer().AsIoBuffer();
+ ZEN_ASSERT(RootIoBuffer.GetSize() > 0);
ResponseBuffers.push_back(RootIoBuffer); // Root object
*AttachmentInfo++ = {.PayloadSize = RootIoBuffer.Size(), .Flags = CbAttachmentEntry::kIsObject, .AttachmentHash = Data.GetObjectHash()};
@@ -257,6 +259,7 @@ FormatPackageMessage(const CbPackage& Data, FormatFlags Flags, void* TargetProce
for (const SharedBuffer& Segment : Compressed.GetSegments())
{
+ ZEN_ASSERT(Segment.GetSize() > 0);
ResponseBuffers.push_back(Segment.AsIoBuffer());
}
}
@@ -264,6 +267,7 @@ FormatPackageMessage(const CbPackage& Data, FormatFlags Flags, void* TargetProce
else if (CbObject AttachmentObject = Attachment.AsObject())
{
IoBuffer ObjIoBuffer = AttachmentObject.GetBuffer().AsIoBuffer();
+ ZEN_ASSERT(ObjIoBuffer.GetSize() > 0);
ResponseBuffers.push_back(ObjIoBuffer);
*AttachmentInfo++ = {.PayloadSize = ObjIoBuffer.Size(),
@@ -307,6 +311,7 @@ FormatPackageMessage(const CbPackage& Data, FormatFlags Flags, void* TargetProce
for (const SharedBuffer& Segment : AttachmentBinary.GetSegments())
{
+ ZEN_ASSERT(Segment.GetSize() > 0);
ResponseBuffers.push_back(Segment.AsIoBuffer());
}
}
@@ -808,6 +813,13 @@ TEST_CASE("CbPackage.Serialization")
Reader.Finalize();
}
+TEST_CASE("CbPackage.EmptyObject")
+{
+ CbPackage Pkg;
+ Pkg.SetObject({});
+ std::vector<IoBuffer> Result = FormatPackageMessage(Pkg, nullptr);
+}
+
TEST_CASE("CbPackage.LocalRef")
{
ScopedTemporaryDirectory TempDir;