From 79c44e3223c2bb9c6c49ccefa9cae71f1f2af336 Mon Sep 17 00:00:00 2001
From: Dan Engelbrecht <dan@engelbrecht.com>
Date: Mon, 5 Feb 2024 12:29:43 +0100
Subject: respond with BadRequest result instead of throwing exception on bad
 request input (#648)

---
 src/zenserver/projectstore/projectstore.cpp | 14 +++++++++++--
 src/zenstore/cache/cacherpc.cpp             | 32 ++++++++++++++++++++++-------
 src/zenutil/packageformat.cpp               | 16 +++++++--------
 3 files changed, 45 insertions(+), 17 deletions(-)

(limited to 'src')

diff --git a/src/zenserver/projectstore/projectstore.cpp b/src/zenserver/projectstore/projectstore.cpp
index 6bb543d63..ea219f9b0 100644
--- a/src/zenserver/projectstore/projectstore.cpp
+++ b/src/zenserver/projectstore/projectstore.cpp
@@ -2838,8 +2838,18 @@ ProjectStore::Rpc(HttpServerRequest&	 HttpReq,
 			}
 			break;
 		case HttpContentType::kCbPackage:
-			Package = ParsePackageMessage(Payload);
-			Cb		= Package.GetObject();
+			try
+			{
+				Package = ParsePackageMessage(Payload);
+				Cb		= Package.GetObject();
+			}
+			catch (const std::invalid_argument& ex)
+			{
+				HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+									  HttpContentType::kText,
+									  fmt::format("Failed to parse package request, reason: '{}'", ex.what()));
+				return false;
+			}
 			if (!Cb)
 			{
 				HttpReq.WriteResponse(HttpResponseCode::BadRequest,
diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp
index 96b344ee9..5acb2b8c9 100644
--- a/src/zenstore/cache/cacherpc.cpp
+++ b/src/zenstore/cache/cacherpc.cpp
@@ -215,15 +215,33 @@ CacheRpcHandler::HandleRpcRequest(const CacheRequestContext& Context,
 	CbPackage	 Package;
 	CbObjectView Object;
 	CbObject	 ObjectBuffer;
-	if (ContentType == ZenContentType::kCbObject)
+	try
 	{
-		ObjectBuffer = LoadCompactBinaryObject(std::move(Body));
-		Object		 = ObjectBuffer;
+		if (ContentType == ZenContentType::kCbObject)
+		{
+			ObjectBuffer = LoadCompactBinaryObject(std::move(Body));
+			Object		 = ObjectBuffer;
+			if (!Object)
+			{
+				ZEN_WARN("Content format not supported, expected compact binary format")
+				return RpcResponseCode::BadRequest;
+			}
+		}
+		else
+		{
+			Package = ParsePackageMessage(Body);
+			Object	= Package.GetObject();
+			if (!Object)
+			{
+				ZEN_WARN("Content format not supported, expected package message format");
+				return RpcResponseCode::BadRequest;
+			}
+		}
 	}
-	else
+	catch (const std::invalid_argument& ex)
 	{
-		Package = ParsePackageMessage(Body);
-		Object	= Package.GetObject();
+		ZEN_WARN("Invalid rpc message package recevied, reason: '{}'", ex.what());
+		return RpcResponseCode::BadRequest;
 	}
 	OutAcceptMagic	   = Object["Accept"sv].AsUInt32();
 	OutAcceptFlags	   = static_cast<RpcAcceptOptions>(Object["AcceptFlags"sv].AsUInt16(0u));
@@ -1637,4 +1655,4 @@ CacheRpcHandler::WriteGetCacheChunksResponse([[maybe_unused]] const CacheRequest
 	return RpcResponse;
 }
 
-}  // namespace zen
\ No newline at end of file
+}  // namespace zen
diff --git a/src/zenutil/packageformat.cpp b/src/zenutil/packageformat.cpp
index 015782283..7c284a4e6 100644
--- a/src/zenutil/packageformat.cpp
+++ b/src/zenutil/packageformat.cpp
@@ -362,7 +362,7 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 
 	if (Hdr.HeaderMagic != kCbPkgMagic)
 	{
-		throw std::runtime_error("invalid CbPackage header magic");
+		throw std::invalid_argument("invalid CbPackage header magic");
 	}
 
 	const uint32_t ChunkCount = Hdr.AttachmentCount + 1;
@@ -455,11 +455,11 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 			CompressedBuffer CompBuf(CompressedBuffer::FromCompressedNoValidate(std::move(ChunkReference)));
 			if (!CompBuf)
 			{
-				throw std::runtime_error(fmt::format("invalid format for chunk #{} at '{}' (offset {}, size {})",
-													 i,
-													 Path,
-													 AttachRefHdr->PayloadByteOffset,
-													 AttachRefHdr->PayloadByteSize));
+				throw std::invalid_argument(fmt::format("invalid format for chunk #{} at '{}' (offset {}, size {})",
+														i,
+														Path,
+														AttachRefHdr->PayloadByteOffset,
+														AttachRefHdr->PayloadByteSize));
 			}
 			Attachments.emplace_back(CbAttachment(std::move(CompBuf), Entry.AttachmentHash));
 		}
@@ -472,7 +472,7 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 					CompressedBuffer CompBuf(CompressedBuffer::FromCompressedNoValidate(IoBuffer(AttachmentBuffer)));
 					if (!CompBuf)
 					{
-						throw std::runtime_error(fmt::format("invalid format for chunk #{} expected compressed buffer for CbObject", i));
+						throw std::invalid_argument(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)));
@@ -487,7 +487,7 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 				CompressedBuffer CompBuf(CompressedBuffer::FromCompressedNoValidate(IoBuffer(AttachmentBuffer)));
 				if (!CompBuf)
 				{
-					throw std::runtime_error(fmt::format("invalid format for chunk #{} expected compressed buffer for attachment", i));
+					throw std::invalid_argument(fmt::format("invalid format for chunk #{} expected compressed buffer for attachment", i));
 				}
 				Attachments.emplace_back(CbAttachment(std::move(CompBuf), Entry.AttachmentHash));
 			}
-- 
cgit v1.2.3