From cd87b4d48abecf310dd88f72063f8f17abfb3421 Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 14:52:52 -0700 Subject: [PATCH 1/6] Fix POST/PUT request bodies being lost server-side LLHttpParse only set contentPresent/partialContent in the HTTP response parsing branch. The request parsing branch never set them, so LLHttpServer dispatched POST/PUT requests with an empty body even though header.contentLength was correct. Also fix the partial-content accumulation in LLHttpServer: the parser was given the receive buffer base pointer but only the length of the latest TCP segment, so requests whose headers and body arrive in separate segments (e.g. from .NET HttpClient) never parsed completely. The null terminator was also being appended one byte past the received data, and the receive pointer shift now matches the accumulated length semantics. Reserve one byte of the receive buffer for the terminator. Repair the CMake test build after the src/Ar restructure (library include dirs, example subdirectory path, stale relative includes) and add tests covering request bodies and a POSTed JSON body split across two TCP segments. Co-Authored-By: Claude Fable 5 --- CMakeLists.txt | 9 +++- example/CppProject/example.cpp | 4 +- src/Ar/LLHttp/HttpParse.c | 18 +++++-- src/Ar/LLHttp/HttpServer.c | 15 +++--- test/tests.cpp | 94 +++++++++++++++++++++++++++++++++- 5 files changed, 124 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 92e302c..1ec7408 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/example/CppProject/example.cpp b/example/CppProject/example.cpp index f36f0c7..c4b85b4 100644 --- a/example/CppProject/example.cpp +++ b/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 } diff --git a/src/Ar/LLHttp/HttpParse.c b/src/Ar/LLHttp/HttpParse.c index 6f0d905..3c1ad34 100644 --- a/src/Ar/LLHttp/HttpParse.c +++ b/src/Ar/LLHttp/HttpParse.c @@ -239,13 +239,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) { diff --git a/src/Ar/LLHttp/HttpServer.c b/src/Ar/LLHttp/HttpServer.c index 73d0fb2..e741152 100644 --- a/src/Ar/LLHttp/HttpServer.c +++ b/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; @@ -166,8 +166,11 @@ void LLHttpServer(LLHttpServer_typ* t) { } else { client->tcpStream.IN.CMD.AcknowledgeData = 1; - client->recvLength = client->tcpStream.OUT.ReceivedDataLength; - memset(((UDINT)client->pReceiveData)+client->recvLength+1, 0, 1); // Append null char + // 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.IN.PAR.pReceiveData - client->pReceiveData) + client->tcpStream.OUT.ReceivedDataLength; + 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 +179,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); diff --git a/test/tests.cpp b/test/tests.cpp index 16def5c..e163f2e 100644 --- a/test/tests.cpp +++ b/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); @@ -342,4 +360,76 @@ TEST_CASE( "Test HTTP Partial Packets", "[LLHttp]") { "" )); +} + +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); + + // Simulate an incoming connection + server.internal.tcpMgr.OUT.NewConnectionAvailable = 1; + server.internal.tcpMgr.OUT.Connection.Ident = 1; + server.internal.tcpMgr.OUT.Connection.UID = 1; + + // Second cycle: connection is accepted and the first segment (headers only) is received + strcpy((char*)client->pReceiveData, headerSegment); + client->tcpStream.Internal.FUB.Receive.recvlen = strlen(headerSegment); + LLHttpServer(&server); + + CHECK(server.error == false); + CHECK(client->parser.partialContent == true); + CHECK(serverCallbackCount == 0); // Request should not be dispatched until the body arrives + + // Third 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)); + } \ No newline at end of file From 8b73d323d0267ee8cdfae8cdfa93fa586a51ddde Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 14:55:35 -0700 Subject: [PATCH 2/6] Update Catch2 to v3.4.0 so tests build with current GCC Co-Authored-By: Claude Fable 5 --- test/CMakeLists.txt | 2 +- test/tests.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 908e566..21fc978 100644 --- a/test/CMakeLists.txt +++ b/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) diff --git a/test/tests.cpp b/test/tests.cpp index e163f2e..4604a20 100644 --- a/test/tests.cpp +++ b/test/tests.cpp @@ -214,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)); } } From d697eebb81fb546279dcb26503472feb442f7796 Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 15:02:08 -0700 Subject: [PATCH 3/6] Add brsstrlen fallback for non-AR builds Co-Authored-By: Claude Fable 5 --- src/Ar/LLHttp/HttpParse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Ar/LLHttp/HttpParse.c b/src/Ar/LLHttp/HttpParse.c index 3c1ad34..5e09810 100644 --- a/src/Ar/LLHttp/HttpParse.c +++ b/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)) From f730cdefbde35e2a8f4d773c9f1b5b20666f512e Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 15:05:33 -0700 Subject: [PATCH 4/6] Zero-initialize server client allocation TMP_alloc memory is not initialized, so the internal client state (TCP stream state, FUB statuses, receive lengths) started as garbage. Co-Authored-By: Claude Fable 5 --- src/Ar/LLHttp/HttpServer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Ar/LLHttp/HttpServer.c b/src/Ar/LLHttp/HttpServer.c index e741152..95a88aa 100644 --- a/src/Ar/LLHttp/HttpServer.c +++ b/src/Ar/LLHttp/HttpServer.c @@ -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++) { From 2bf1f9be06e15489403148edc9b1d3ac9cd95e35 Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 15:11:17 -0700 Subject: [PATCH 5/6] Guard receive length accumulation and drive server test via stub connection manager Co-Authored-By: Claude Fable 5 --- src/Ar/LLHttp/HttpServer.c | 5 ++++- test/tests.cpp | 19 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Ar/LLHttp/HttpServer.c b/src/Ar/LLHttp/HttpServer.c index 95a88aa..3dfb8fb 100644 --- a/src/Ar/LLHttp/HttpServer.c +++ b/src/Ar/LLHttp/HttpServer.c @@ -170,7 +170,10 @@ void LLHttpServer(LLHttpServer_typ* t) { // 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.IN.PAR.pReceiveData - client->pReceiveData) + client->tcpStream.OUT.ReceivedDataLength; + client->recvLength = client->tcpStream.OUT.ReceivedDataLength; + 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; diff --git a/test/tests.cpp b/test/tests.cpp index 4604a20..7a40954 100644 --- a/test/tests.cpp +++ b/test/tests.cpp @@ -405,21 +405,24 @@ TEST_CASE( "Test HTTP Server POST body split across TCP segments", "[LLHttp]") { handler.newMessageCallback = (UDINT)&serverNewMessageCallback; REQUIRE(LLHttpAddHandler(server.ident, (UDINT)&handler) == 0); - // Simulate an incoming connection - server.internal.tcpMgr.OUT.NewConnectionAvailable = 1; - server.internal.tcpMgr.OUT.Connection.Ident = 1; - server.internal.tcpMgr.OUT.Connection.UID = 1; - - // Second cycle: connection is accepted and the first segment (headers only) is received + // 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); - LLHttpServer(&server); + + // 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 - // Third cycle: the second segment (body) is received at the shifted receive pointer + // 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); From a5932c14743d59773e332e137fede27143560b47 Mon Sep 17 00:00:00 2001 From: Scott Claiborne Date: Thu, 11 Jun 2026 15:15:08 -0700 Subject: [PATCH 6/6] Create test report directory so ctest runs on a clean checkout Co-Authored-By: Claude Fable 5 --- test/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 21fc978..9aef927 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -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/)