From 5a8b8e45fc9f0e115a277c0f3a3b022bdb24cafc Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Mon, 12 Jan 2026 12:32:04 +0000 Subject: [PATCH 1/5] Return HTTPSuccess when HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG is set When the HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG is set, the HTTP_PARSING_INCOMPLETE state is expected and valid, as the body data may not yet be received from the transport layer. Previously, this scenario incorrectly returned HTTPInsufficientMemory. This commit corrects the behavior to return HTTPSuccess instead, properly handling the case where body parsing is intentionally deferred. Fixes: https://github.com/FreeRTOS/coreHTTP/issues/193 Signed-off-by: Gaurav Aggarwal --- source/core_http_client.c | 43 ++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 8ce43b0f..c592f0e4 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -137,7 +137,7 @@ static HTTPStatus_t addRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, * @param[in] parsingState State of the parsing on the HTTP response. * @param[in] totalReceived The amount of network data received in the response * buffer. - * @param[in] responseBufferLen The length of the response buffer. + * @param[in] pResponse The response information. * * @return Returns #HTTPSuccess if the parsing state is complete. If * the parsing state denotes it never started, then return #HTTPNoResponse. If @@ -147,7 +147,7 @@ static HTTPStatus_t addRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, */ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, size_t totalReceived, - size_t responseBufferLen ); + const HTTPResponse_t * pResponse ); /** * @brief Send the HTTP request over the network. @@ -1984,13 +1984,13 @@ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, size_t totalReceived, - size_t responseBufferLen ) + const HTTPResponse_t * pResponse ) { HTTPStatus_t returnStatus = HTTPSuccess; assert( parsingState >= HTTP_PARSING_NONE && parsingState <= HTTP_PARSING_COMPLETE ); - assert( totalReceived <= responseBufferLen ); + assert( totalReceived <= pResponse->bufferLen ); /* If no parsing occurred, that means network data was never received. */ if( parsingState == HTTP_PARSING_NONE ) @@ -2002,21 +2002,26 @@ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, } else if( parsingState == HTTP_PARSING_INCOMPLETE ) { - if( totalReceived == responseBufferLen ) + /* HTTP_PARSING_INCOMPLETE is okay when HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG is set + * as the body data may yet to be read from the transport. */ + if( ( pResponse->respOptionFlags & HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG ) == 0 ) { - LogError( ( "Cannot receive complete response from transport" - " interface: Response buffer has insufficient " - "space: responseBufferLen=%lu", - ( unsigned long ) responseBufferLen ) ); - returnStatus = HTTPInsufficientMemory; - } - else - { - LogError( ( "Received partial response from transport " - "receive(): ResponseSize=%lu, TotalBufferSize=%lu", - ( unsigned long ) totalReceived, - ( unsigned long ) ( responseBufferLen - totalReceived ) ) ); - returnStatus = HTTPPartialResponse; + if( totalReceived == pResponse->bufferLen ) + { + LogError( ( "Cannot receive complete response from transport" + " interface: Response buffer has insufficient " + "space: responseBufferLen=%lu", + ( unsigned long ) pResponse->bufferLen ) ); + returnStatus = HTTPInsufficientMemory; + } + else + { + LogError( ( "Received partial response from transport " + "receive(): ResponseSize=%lu, TotalBufferSize=%lu", + ( unsigned long ) totalReceived, + ( unsigned long ) ( pResponse->bufferLen - totalReceived ) ) ); + returnStatus = HTTPPartialResponse; + } } } else @@ -2155,7 +2160,7 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t * the parsing and how much data is in the buffer. */ returnStatus = getFinalResponseStatus( parsingContext.state, totalReceived, - pResponse->bufferLen ); + pResponse ); } return returnStatus; From aeefcca9083f17b52829eaf4799574a75f7b58fb Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Wed, 14 Jan 2026 05:42:25 +0000 Subject: [PATCH 2/5] Fix unit tests and coverage Signed-off-by: Gaurav Aggarwal --- test/unit-test/core_http_send_utest.c | 68 +++++++++++++++++++++++++++ tools/cmock/coverage.cmake | 12 +++-- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index 48041d72..808c1efc 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -760,6 +760,32 @@ static llhttp_errno_t llhttp_execute_partial_body( llhttp_t * pParser, return HPE_OK; } + +/* Mocked llhttp_execute callback that will be called when partial body has been + * received from the network. It returns HPE_PAUSED to mimic stopping parsing + * the body because user set HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG. */ +static llhttp_errno_t llhttp_execute_partial_body_do_not_parse( llhttp_t * pParser, + const char * pData, + size_t len, + int cmock_num_calls ) +{ + ( void ) cmock_num_calls; + ( void ) len; + + const char * pNext = pData; + llhttp_settings_t * pSettings = ( llhttp_settings_t * ) pParser->settings; + + pSettings->on_message_begin( pParser ); + + helper_parse_status_line( &pNext, pParser, pSettings ); + helper_parse_headers( &pNext, pParser, pSettings ); + helper_parse_headers_finish( &pNext, pParser, pSettings, NULL ); + + pParser->error_pos = pNext; + pParser->error = HPE_PAUSED; + return HPE_PAUSED; +} + /* Mocked llhttp_execute callback that will be on a response of type * transfer-encoding chunked. */ static llhttp_errno_t llhttp_execute_chunked_body( llhttp_t * pParser, @@ -768,6 +794,7 @@ static llhttp_errno_t llhttp_execute_chunked_body( llhttp_t * pParser, int cmock_num_calls ) { ( void ) cmock_num_calls; + ( void ) len; const char * pNext = pData; uint8_t isHeadResponse = 0; @@ -1158,6 +1185,47 @@ void test_HTTPClient_Send_parse_partial_body( void ) /*-----------------------------------------------------------*/ +/* Test successfully parsing a response where up to the middle of the body + * is received on the first network read, then the rest of the response is not + * received from the network because HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG is set. + */ +void test_HTTPClient_Send_do_not_parse_partial_body( void ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + llhttp_execute_Stub( llhttp_execute_partial_body_do_not_parse ); + + memcpy( requestHeaders.pBuffer, + HTTP_TEST_REQUEST_GET_HEADERS, + HTTP_TEST_REQUEST_GET_HEADERS_LENGTH ); + requestHeaders.headersLen = HTTP_TEST_REQUEST_GET_HEADERS_LENGTH; + pNetworkData = ( uint8_t * ) HTTP_TEST_RESPONSE_GET; + networkDataLen = HTTP_TEST_RESPONSE_GET_LENGTH; + firstPartBytes = HTTP_TEST_RESPONSE_GET_PARTIAL_BODY_LENGTH; + + response.respOptionFlags |= HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG; + + returnStatus = HTTPClient_Send( &transportInterface, + &requestHeaders, + NULL, + 0, + &response, + 0 ); + TEST_ASSERT_EQUAL( HTTPSuccess, returnStatus ); + TEST_ASSERT_EQUAL( response.pBuffer + ( sizeof( HTTP_STATUS_LINE_OK ) - 1 ), response.pHeaders ); + TEST_ASSERT_EQUAL( HTTP_TEST_RESPONSE_GET_HEADERS_LENGTH - HTTP_HEADER_END_INDICATOR_LEN, + response.headersLen ); + TEST_ASSERT_EQUAL( response.pHeaders + HTTP_TEST_RESPONSE_GET_HEADERS_LENGTH, response.pBody ); + TEST_ASSERT_EQUAL( HTTP_TEST_RESPONSE_GET_PARTIAL_BODY_LENGTH - HTTP_TEST_RESPONSE_HEAD_LENGTH, response.bodyLen ); + TEST_ASSERT_EQUAL( HTTP_STATUS_CODE_OK, response.statusCode ); + TEST_ASSERT_EQUAL( HTTP_TEST_RESPONSE_GET_CONTENT_LENGTH, response.contentLength ); + TEST_ASSERT_EQUAL( HTTP_TEST_RESPONSE_GET_HEADER_COUNT, response.headerCount ); + TEST_ASSERT_BITS_HIGH( HTTP_RESPONSE_CONNECTION_CLOSE_FLAG, response.respFlags ); + TEST_ASSERT_BITS_LOW( HTTP_RESPONSE_CONNECTION_KEEP_ALIVE_FLAG, response.respFlags ); +} + +/*-----------------------------------------------------------*/ + /* Test receiving a response where the body is of Transfer-Encoding chunked. */ void test_HTTPClient_Send_parse_chunked_body( void ) { diff --git a/tools/cmock/coverage.cmake b/tools/cmock/coverage.cmake index e035ea4c..09616fcd 100644 --- a/tools/cmock/coverage.cmake +++ b/tools/cmock/coverage.cmake @@ -14,9 +14,9 @@ execute_process( COMMAND lcov --directory ${CMAKE_BINARY_DIR} --base-directory ${CMAKE_BINARY_DIR} --initial --capture - --rc lcov_branch_coverage=1 - --rc genhtml_branch_coverage=1 + --rc branch_coverage=1 --output-file=${CMAKE_BINARY_DIR}/base_coverage.info + --include "*source*" ) file(GLOB files "${CMAKE_BINARY_DIR}/bin/tests/*") @@ -46,10 +46,10 @@ execute_process(COMMAND ruby execute_process( COMMAND lcov --capture --rc lcov_branch_coverage=1 - --rc genhtml_branch_coverage=1 --base-directory ${CMAKE_BINARY_DIR} --directory ${CMAKE_BINARY_DIR} --output-file ${CMAKE_BINARY_DIR}/second_coverage.info + --include "*source*" ) # combile baseline results (zeros) with the one after running the tests @@ -60,10 +60,12 @@ execute_process( --add-tracefile ${CMAKE_BINARY_DIR}/second_coverage.info --output-file ${CMAKE_BINARY_DIR}/coverage.info --no-external - --rc lcov_branch_coverage=1 + --rc branch_coverage=1 + --include "*source*" + --exclude "*dependency*" ) execute_process( - COMMAND genhtml --rc lcov_branch_coverage=1 + COMMAND genhtml --rc branch_coverage=1 --branch-coverage --output-directory ${CMAKE_BINARY_DIR}/coverage ${CMAKE_BINARY_DIR}/coverage.info From 4a5da118d652448e4c7fe3c9cd4c1d1b8bd0b538 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Wed, 14 Jan 2026 05:43:49 +0000 Subject: [PATCH 3/5] Fix formatting check Signed-off-by: Gaurav Aggarwal --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b21f1f7..e63afbcd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,7 @@ jobs: path: ./ formatting: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - name: Check formatting From 17b966fcd99c04a623544fd5297e79d8c45201a2 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Wed, 14 Jan 2026 05:49:19 +0000 Subject: [PATCH 4/5] Fix SSOT check Signed-off-by: Gaurav Aggarwal --- source/interface/transport_interface.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source/interface/transport_interface.h b/source/interface/transport_interface.h index bdbb271a..ca0e4574 100644 --- a/source/interface/transport_interface.h +++ b/source/interface/transport_interface.h @@ -96,6 +96,7 @@ * without TLS, it is typically implemented by calling the TCP layer receive * function. @ref TransportRecv_t may be invoked multiple times by the protocol * library, if fewer bytes than were requested to receive are returned. + * Please note that it is HIGHLY RECOMMENDED that the transport receive implementation does NOT block. *

* Example code: * @code{c} @@ -200,6 +201,9 @@ typedef struct NetworkContext NetworkContext_t; * coreMQTT will continue to call the transport interface if it receives * a partial packet until it accumulates enough data to get the complete * MQTT packet. + * A non‐blocking implementation is also essential so that the library's inbuilt + * keep‐alive mechanism can work properly, given the user chooses to use + * that over their own keep alive mechanism. * * @param[in] pNetworkContext Implementation-defined network context. * @param[in] pBuffer Buffer to receive the data into. From 89b0cac1b0bb1f5e5b9cda806b6694f526fc0b07 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Wed, 14 Jan 2026 05:53:32 +0000 Subject: [PATCH 5/5] Fix coverage check Signed-off-by: Gaurav Aggarwal --- .github/workflows/ci.yml | 5 ++--- tools/cmock/coverage.cmake | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e63afbcd..ef4d6c49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,9 +34,8 @@ jobs: - name: Run Coverage run: | make -C build/ coverage - declare -a EXCLUDE=("\*test\*" "\*CMakeCCompilerId\*" "\*mocks\*" "\*3rdparty\*") - echo ${EXCLUDE[@]} | xargs lcov --rc lcov_branch_coverage=1 -r build/coverage.info -o build/coverage.info - lcov --rc lcov_branch_coverage=1 --list build/coverage.info + lcov --rc branch_coverage=1 -r build/coverage.info -o build/coverage.info + lcov --rc branch_coverage=1 --list build/coverage.info - name: Check Coverage uses: FreeRTOS/CI-CD-Github-Actions/coverage-cop@main with: diff --git a/tools/cmock/coverage.cmake b/tools/cmock/coverage.cmake index 09616fcd..381c06d3 100644 --- a/tools/cmock/coverage.cmake +++ b/tools/cmock/coverage.cmake @@ -45,7 +45,7 @@ execute_process(COMMAND ruby # capture data after running the tests execute_process( COMMAND lcov --capture - --rc lcov_branch_coverage=1 + --rc branch_coverage=1 --base-directory ${CMAKE_BINARY_DIR} --directory ${CMAKE_BINARY_DIR} --output-file ${CMAKE_BINARY_DIR}/second_coverage.info