Fix POST/PUT request bodies being lost server-side

CMakeLists.txt

@@ -14,9 +14,16 @@ add_definitions(-D_BUR_FORMAT_BRELF)
 add_definitions(-D_REPLACE_CONST)
 
 add_library(${PROJECT_NAME} ${LLHttp_SRC})
+
+target_include_directories(${PROJECT_NAME} PUBLIC
+    src/Ar/LLHttp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../../includes
+    ${CMAKE_CURRENT_SOURCE_DIR}/../../includes/loupe/Includes
+)
+
 enable_testing ()
 add_subdirectory (test)
-add_subdirectory (example)
+add_subdirectory (example/CppProject)
 
 target_link_libraries(LLHttp PUBLIC TCPComm)
 target_link_libraries(LLHttp PUBLIC StringExt)

example/CppProject/example.cpp

@@ -9,8 +9,8 @@
 extern "C" {
 #endif
 
-#include "../LLHttpH.h"
-#include "../HttpUtility.h"
+#include "LLHttpH.h"
+#include "HttpUtility.h"
 
 #ifdef __cplusplus
 }

src/Ar/LLHttp/HttpParse.c

@@ -95,6 +95,9 @@ size_t ftoa(float n, char* res, int afterpoint)
 #ifndef brsatoi
 #define brsatoi(a) atoi((char*)a)
 #endif
+#ifndef brsstrlen
+#define brsstrlen(a) strlen((char*)a)
+#endif
 
 #define min(a,b) (((a)<(b))?(a):(b))
 
@@ -239,13 +242,21 @@ void LLHttpParse(LLHttpParse_typ* t) {
 
 			copyHeaderLine(&t->header.lines[index], &headerLines[index]);
 		}
-
-
+
+		// We have content if content length is non-zero
+		// Its possible that we still do not have content, e.g. if the headers lied. We do not have anyway to know this
+		// See spec at https://greenbytes.de/tech/webdav/rfc7230.html#message.body
+		if(t->header.contentLength != 0) {
+			// We have content (maybe ;)
+			t->contentPresent = 1;
+			t->partialContent = (returnValue+t->header.contentLength > t->dataLength);
+		}
+
 	}
-	
-	
+
+
 	// Parse first line
-	
+
 }
 
 signed short LLHttpgetHeaderIndex(unsigned long headerlines, unsigned long name, unsigned long value) {

src/Ar/LLHttp/HttpServer.c

@@ -38,7 +38,7 @@ void HttpShiftReceivePointer(LLHttpServerInternalClient_typ* client, unsigned lo
 }
 void HttpResetReceivePointer(LLHttpServerInternalClient_typ* client) {
 	client->tcpStream.IN.PAR.pReceiveData = client->pReceiveData;
-	client->tcpStream.IN.PAR.MaxReceiveLength = client->receiveDataSize;
+	client->tcpStream.IN.PAR.MaxReceiveLength = client->receiveDataSize - 1; // Reserve a byte for the appended null char
 }
 void HttpConnect(LLHttpServerInternalClient_typ* client, TCPConnection_Desc_typ* connection) {
 	if(!client || !connection) return;
@@ -75,6 +75,7 @@ void HttpServerSetError(LLHttpServer_typ* t, LLHttpServerInternalClient_typ* cli
 
 void LLHttpServerInit(LLHttpServer_typ* t) {
 	TMP_alloc(sizeof(LLHttpServerInternalClient_typ)*t->numClients, &t->internal.pClients);
+	memset(t->internal.pClients, 0, sizeof(LLHttpServerInternalClient_typ)*t->numClients); // TMP_alloc memory is not initialized
 	t->internal.numClients = t->numClients;
 	int index;
 	for (index = 0; index < t->internal.numClients; index++) {
@@ -166,8 +167,14 @@ void LLHttpServer(LLHttpServer_typ* t) {
 			}
 			else {
 				client->tcpStream.IN.CMD.AcknowledgeData = 1;
+				// pReceiveData is the base of the receive buffer. The TCP receive pointer has been
+				// shifted past any previously received partial data, so the total received length
+				// is the shift offset plus the newly received length
 				client->recvLength = client->tcpStream.OUT.ReceivedDataLength;
-				memset(((UDINT)client->pReceiveData)+client->recvLength+1, 0, 1); // Append null char
+				if(client->tcpStream.IN.PAR.pReceiveData > client->pReceiveData) {
+					client->recvLength += client->tcpStream.IN.PAR.pReceiveData - client->pReceiveData;
+				}
+				memset(((UDINT)client->pReceiveData)+client->recvLength, 0, 1); // Append null char
 				// TODO: Parse 
 				client->parser.data = client->pReceiveData;
 				client->parser.dataLength = client->recvLength;
@@ -176,9 +183,9 @@ void LLHttpServer(LLHttpServer_typ* t) {
 				// TODO: Check for partial packet
 				if(client->parser.partialPacket || (client->parser.partialContent)) { // TODO: We should handle expect: 100
 					// TODO: Handle partial packets...
-					
-					// Shift pointer
-					HttpShiftReceivePointer(client, client->recvLength);
+
+					// Shift pointer past the newly received data so the next segment is appended
+					HttpShiftReceivePointer(client, client->tcpStream.OUT.ReceivedDataLength);
 				}
 				else if(client->parser.error) {
 					HttpServerSetError(t, client, client->parser.errorId);

test/CMakeLists.txt

@@ -21,7 +21,7 @@ Include(FetchContent)
 FetchContent_Declare(
 Catch2
 GIT_REPOSITORY https://github.com/catchorg/Catch2.git
-GIT_TAG        v3.0.0-preview3
+GIT_TAG        v3.4.0
 )
 
 # set(CMAKE_BUILD_TYPE Debug)
@@ -33,6 +33,7 @@ target_link_libraries(LLHttp_test PRIVATE Catch2::Catch2WithMain)
 list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
 include(CTest)
 include(Catch)
+file(MAKE_DIRECTORY ${PROJECT_SOURCE_DIR}/report)
 catch_discover_tests(LLHttp_test OUTPUT_DIR ${PROJECT_SOURCE_DIR}/report)
 
 target_include_directories(LLHttp_test PUBLIC ${PROJECT_SOURCE_DIR}/../../includes/)

test/tests.cpp

@@ -9,8 +9,8 @@
 extern "C" {
 #endif
 
-#include "../LLHttpH.h"
-#include "../HttpUtility.h"
+#include "LLHttpH.h"
+#include "HttpUtility.h"
 
 #ifdef __cplusplus
 }
@@ -91,6 +91,24 @@ TEST_CASE( "Test HTTP Parser", "[LLHttp]" ) {
 		CHECK_THAT(parser.header.uri, Catch::Matchers::Equals("/"));
 	}
 
+	SECTION( "Parse simple request with basic body" ) {
+		ParseTest("POST /api/data HTTP/1.1\r\ncontent-length: 6\r\ncontent-type: text\r\n\r\nsimple");
+		CHECK(parser.error == false);
+		CHECK(parser.header.method == LLHTTP_METHOD_POST);
+		CHECK(parser.contentPresent == true);
+		CHECK(parser.partialContent == false);
+		CHECK(parser.header.contentLength == 6);
+		CHECK_THAT((char*)parser.content, Catch::Matchers::Equals("simple"));
+		CHECK_THAT((char*)parser.header.contentType, Catch::Matchers::Equals("text"));
+	}
+
+	SECTION( "Parse request with partial body" ) {
+		ParseTest("POST /api/data HTTP/1.1\r\ncontent-length: 12\r\n\r\nsimple");
+		CHECK(parser.error == false);
+		CHECK(parser.contentPresent == true);
+		CHECK(parser.partialContent == true);
+	}
+
 	SECTION( "Parse simple response with basic body" ) {
 		ParseTest("HTTP/1.0 200 OK\r\ncontent-length: 6\r\ncontent-type: text\r\n\r\nsimple");
 		CHECK(parser.error == false);
@@ -196,9 +214,9 @@ TEST_CASE( "Test HTTP Build Response", "[LLHttp]" ) {
 		// HTTP/1.1 200 OK\r\ncontent-length: 6\r\ncontent-type: text\r\n\r\nsimple
 		CHECK_THAT(buffer, Catch::Matchers::StartsWith("HTTP/1.1 200 OK"));
 		CHECK_THAT(buffer, Catch::Matchers::EndsWith("\r\n\r\nsimple"));
-		CHECK_THAT(buffer, Catch::Matchers::Contains(contentType));
-		CHECK_THAT(buffer, Catch::Matchers::Contains(contentLength));
-		CHECK_THAT(buffer, Catch::Matchers::Contains(date));
+		CHECK_THAT(buffer, Catch::Matchers::ContainsSubstring(contentType));
+		CHECK_THAT(buffer, Catch::Matchers::ContainsSubstring(contentLength));
+		CHECK_THAT(buffer, Catch::Matchers::ContainsSubstring(date));
 	}
 }
 
@@ -342,4 +360,79 @@ TEST_CASE( "Test HTTP Partial Packets", "[LLHttp]") {
 				"</html>"
 				));
 
+}
+
+static int serverCallbackCount = 0;
+static char serverCallbackBody[500];
+static LLHttpHeader_typ serverCallbackHeader;
+
+static void serverNewMessageCallback(UDINT context, LLHttpServiceLink_typ* api, LLHttpHeader_typ* header, unsigned char* data) {
+	serverCallbackCount++;
+	memcpy(&serverCallbackHeader, header, sizeof(serverCallbackHeader));
+	strncpy(serverCallbackBody, (char*)data, sizeof(serverCallbackBody)-1);
+}
+
+TEST_CASE( "Test HTTP Server POST body split across TCP segments", "[LLHttp]") {
+
+	LLHttpServer_typ server = {};
+
+	// .NET HttpClient (among others) sends the headers and the body of a POST in
+	// separate writes, so the server receives them as two TCP segments
+	const char* headerSegment =
+				"POST /api/data HTTP/1.1\r\n"\
+				"Host: plc.local\r\n"\
+				"content-type: application/json\r\n"\
+				"content-length: 27\r\n"\
+				"\r\n";
+	const char* bodySegment = "{\"name\":\"test\",\"value\":420}";
+	REQUIRE(strlen(bodySegment) == 27);
+
+	server.enable = true;
+	server.numClients = LLHTTP_MAX_NUM_CLIENTS;
+	server.bufferSize = 2000;
+
+	// First cycle initializes internals and allocates the client receive buffers
+	LLHttpServer(&server);
+
+	LLHttpServerInternalClient_typ* client = &server.internal.pClients[0];
+
+	// Register a handler for the POST request
+	serverCallbackCount = 0;
+	memset(serverCallbackBody, 0, sizeof(serverCallbackBody));
+	LLHttpHandler_typ handler = {};
+	handler.method = LLHTTP_METHOD_POST;
+	strcpy(handler.uri, "/api/data");
+	handler.newMessageCallback = (UDINT)&serverNewMessageCallback;
+	REQUIRE(LLHttpAddHandler(server.ident, (UDINT)&handler) == 0);
+
+	// Preload the first TCP segment (headers only). The stubbed TCP layer reports received
+	// data on every cycle, so the data must already be in place when the stubbed connection
+	// manager accepts the incoming connection
+	strcpy((char*)client->pReceiveData, headerSegment);
+	client->tcpStream.Internal.FUB.Receive.recvlen = strlen(headerSegment);
+
+	// Cycle until the connection is accepted and the first segment has been received
+	int cycles = 0;
+	while(!client->connected && ++cycles < 10) {
+		LLHttpServer(&server);
+	}
+	REQUIRE(client->connected == 1);
+
+	CHECK(server.error == false);
+	CHECK(client->parser.partialContent == true);
+	CHECK(serverCallbackCount == 0); // Request should not be dispatched until the body arrives
+
+	// Next cycle: the second segment (body) is received at the shifted receive pointer
+	strcpy((char*)client->tcpStream.IN.PAR.pReceiveData, bodySegment);
+	client->tcpStream.Internal.FUB.Receive.recvlen = strlen(bodySegment);
+	LLHttpServer(&server);
+
+	CHECK(server.error == false);
+	CHECK(serverCallbackCount == 1);
+	CHECK(serverCallbackHeader.method == LLHTTP_METHOD_POST);
+	CHECK_THAT(serverCallbackHeader.uri, Catch::Matchers::Equals("/api/data"));
+	CHECK(serverCallbackHeader.contentLength == strlen(bodySegment));
+	CHECK_THAT(serverCallbackBody, Catch::Matchers::Equals(bodySegment));
+	CHECK_THAT((char*)&client->pCurrentRequest->contentStart, Catch::Matchers::Equals(bodySegment));
+
 }