Changed interface for httpServerRequest::SessionId()/RequestId() so they share storage and lazy eval logic

They now call into ParseSessionId()/ParseRequestId() when required

Eliminates redundant logic in derived implementations

Also moved package transport code into httpshared.(cpp|h) for easier sharing with client code

Added some I/O error reporting in http.sys related code

Changed IHttpPackageHandler interface to support partially updated handling flow

1 files changed, 85 insertions, 54 deletions

diff --git a/zenhttp/httpsys.cpp b/zenhttp/httpsys.cpp
index f5f9d4249..cde371f2f 100644
--- a/zenhttp/httpsys.cpp
+++ b/zenhttp/httpsys.cpp
@@ -4,6 +4,7 @@
 
 #include <zencore/compactbinary.h>
 #include <zencore/compactbinarybuilder.h>
+#include <zencore/compactbinarypackage.h>
 #include <zencore/except.h>
 #include <zencore/logging.h>
 #include <zencore/scopeguard.h>
@@ -145,7 +146,10 @@ private:
 
 /**
  * This is the handler for the initial HTTP I/O request which will receive the headers
- * and however much of the remaining payload might fit in the embedded request buffer
+ * and however much of the remaining payload might fit in the embedded request buffer.
+ *
+ * It is also used to receive any entity body data relating to the request
+ *
  */
 struct InitialRequestHandler : public HttpSysRequestHandler
 {
@@ -176,8 +180,8 @@ public:
 	HttpSysServerRequest(HttpSysTransaction& Tx, HttpService& Service, IoBuffer PayloadBuffer);
 	~HttpSysServerRequest() = default;
 
-	virtual Oid		 SessionId() const override;
-	virtual uint32_t RequestId() const override;
+	virtual Oid		 ParseSessionId() const override;
+	virtual uint32_t ParseRequestId() const override;
 
 	virtual IoBuffer ReadPayload() override;
 	virtual void	 WriteResponse(HttpResponseCode ResponseCode) override;
@@ -409,7 +413,14 @@ HttpSysRequestHandler*
 HttpMessageResponseRequest::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred)
 {
 	ZEN_UNUSED(NumberOfBytesTransferred);
-	ZEN_UNUSED(IoResult);
+
+	if (IoResult)
+	{
+		spdlog::warn("response aborted due to error: '{}'", GetWindowsErrorAsString(IoResult));
+
+		// if one transmit failed there's really no need to go on
+		return nullptr;
+	}
 
 	if (m_RemainingChunkCount == 0)
 	{
@@ -438,6 +449,21 @@ HttpMessageResponseRequest::IssueRequest(std::error_code& ErrorCode)
 	m_RemainingChunkCount -= ThisRequestChunkCount;
 	m_NextDataChunkOffset += ThisRequestChunkCount;
 
+	/* Should this code also use HTTP_SEND_RESPONSE_FLAG_BUFFER_DATA?
+
+		From the docs:
+
+		This flag enables buffering of data in the kernel on a per-response basis. It should
+		be used by an application doing synchronous I/O, or by a an application doing
+		asynchronous I/O with no more than one send outstanding at a time.
+
+		Applications using asynchronous I/O which may have more than one send outstanding at
+		a time should not use this flag.
+
+		When this flag is set, it should be used consistently in calls to the
+		HttpSendHttpResponse function as well.
+	 */
+
 	ULONG SendFlags = 0;
 
 	if (m_RemainingChunkCount)
@@ -526,10 +552,14 @@ HttpMessageResponseRequest::IssueRequest(std::error_code& ErrorCode)
 
 		CancelThreadpoolIo(Iocp);
 
-		spdlog::error("failed to send HTTP response (error: {}) URL: {}"sv, SendResult, HttpReq->pRawUrl);
+		spdlog::error("failed to send HTTP response (error: '{}'), request URL: {}"sv, SendResult, HttpReq->pRawUrl);
 
 		ErrorCode = MakeWin32ErrorCode(SendResult);
 	}
+	else
+	{
+		ErrorCode = {};
+	}
 }
 
 /**
@@ -926,53 +956,66 @@ HttpSysTransaction::InvokeRequestHandler(HttpService& Service, IoBuffer Payload)
 {
 	HttpSysServerRequest& ThisRequest = m_HandlerRequest.emplace(*this, Service, Payload);
 
-	if ((ThisRequest.RequestContentType() == HttpContentType::kCbPackageOffer) && (ThisRequest.RequestVerb() == HttpVerb::kPost))
+	if (ThisRequest.RequestVerb() == HttpVerb::kPost)
 	{
-		// The client is presenting us with a package attachments offer, we need
-		// to filter it down to the list of attachments we need them to send in
-		// the follow-up request
-
-		m_PackageHandler = Service.HandlePackageRequest(ThisRequest);
-
-		if (m_PackageHandler)
+		if (ThisRequest.RequestContentType() == HttpContentType::kCbPackageOffer)
 		{
-			CbObject OfferMessage = LoadCompactBinaryObject(Payload);
+			// The client is presenting us with a package attachments offer, we need
+			// to filter it down to the list of attachments we need them to send in
+			// the follow-up request
 
-			std::vector<IoHash> OfferCids;
+			m_PackageHandler = Service.HandlePackageRequest(ThisRequest);
 
-			for (auto& CidEntry : OfferMessage["offer"])
+			if (m_PackageHandler)
 			{
-				if (!CidEntry.IsHash())
+				CbObject OfferMessage = LoadCompactBinaryObject(Payload);
+
+				std::vector<IoHash> OfferCids;
+
+				for (auto& CidEntry : OfferMessage["offer"])
 				{
-					// Should yield bad request response?
+					if (!CidEntry.IsHash())
+					{
+						// Should yield bad request response?
+
+						continue;
+					}
 
-					continue;
+					OfferCids.push_back(CidEntry.AsHash(IoHash::Zero));
 				}
 
-				OfferCids.push_back(CidEntry.AsHash(IoHash::Zero));
-			}
+				m_PackageHandler->FilterOffer(OfferCids);
 
-			m_PackageHandler->FilterOffer(OfferCids);
+				CbObjectWriter ResponseWriter;
+				ResponseWriter.BeginArray("need");
 
-			CbObjectWriter ResponseWriter;
-			ResponseWriter.BeginArray("need");
+				for (const IoHash& Cid : OfferCids)
+				{
+					ResponseWriter.AddHash(Cid);
+				}
 
-			for (const IoHash& Cid : OfferCids)
-			{
-				ResponseWriter.AddHash(Cid);
+				ResponseWriter.EndArray();
+
+				// Emit filter response
+				ThisRequest.WriteResponse(HttpResponseCode::OK, ResponseWriter.Save());
+
+				return ThisRequest;
 			}
+		}
+		else if (ThisRequest.RequestContentType() == HttpContentType::kCbPackage)
+		{
+			// Process chunks in package request
 
-			ResponseWriter.EndArray();
+			m_PackageHandler = Service.HandlePackageRequest(ThisRequest);
 
-			// Emit filter response
-			ThisRequest.WriteResponse(HttpResponseCode::OK, ResponseWriter.Save());
+			if (m_PackageHandler)
+			{
+				CbPackage Package;
 
-			return ThisRequest;
+				Package.TryLoad(ThisRequest.ReadPayload());
+			}
 		}
 	}
-	else if ((ThisRequest.RequestContentType() == HttpContentType::kCbPackage) && (ThisRequest.RequestVerb() == HttpVerb::kPost))
-	{
-	}
 
 	// Default request handling
 
@@ -1024,13 +1067,8 @@ HttpSysServerRequest::HttpSysServerRequest(HttpSysTransaction& Tx, HttpService&
 }
 
 Oid
-HttpSysServerRequest::SessionId() const
+HttpSysServerRequest::ParseSessionId() const
 {
-	if (m_Flags & kHaveSessionId)
-	{
-		return m_SessionId;
-	}
-
 	const HTTP_REQUEST* HttpRequestPtr = m_HttpTx.HttpRequest();
 
 	for (int i = 0; i < HttpRequestPtr->Headers.UnknownHeaderCount; ++i)
@@ -1042,24 +1080,17 @@ HttpSysServerRequest::SessionId() const
 		{
 			if (Header.RawValueLength == Oid::StringLength)
 			{
-				m_SessionId = Oid::FromHexString({Header.pRawValue, Header.RawValueLength});
+				return Oid::FromHexString({Header.pRawValue, Header.RawValueLength});
 			}
 		}
 	}
 
-	m_Flags |= kHaveSessionId;
-
-	return m_SessionId;
+	return {};
 }
 
 uint32_t
-HttpSysServerRequest::RequestId() const
+HttpSysServerRequest::ParseRequestId() const
 {
-	if (m_Flags & kHaveRequestId)
-	{
-		return m_RequestId;
-	}
-
 	const HTTP_REQUEST* HttpRequestPtr = m_HttpTx.HttpRequest();
 
 	for (int i = 0; i < HttpRequestPtr->Headers.UnknownHeaderCount; ++i)
@@ -1070,13 +1101,13 @@ HttpSysServerRequest::RequestId() const
 		if (HeaderName == "UE-Request"sv)
 		{
 			std::string_view RequestValue{Header.pRawValue, Header.RawValueLength};
-			std::from_chars(RequestValue.data(), RequestValue.data() + RequestValue.size(), m_RequestId);
+			uint32_t		 RequestId = 0;
+			std::from_chars(RequestValue.data(), RequestValue.data() + RequestValue.size(), RequestId);
+			return RequestId;
 		}
 	}
 
-	m_Flags |= kHaveRequestId;
-
-	return m_RequestId;
+	return 0;
 }
 
 IoBuffer