diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b21f1f7..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: @@ -73,7 +72,7 @@ jobs: path: ./ formatting: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - name: Check formatting 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; 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. 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..381c06d3 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/*") @@ -45,11 +45,11 @@ execute_process(COMMAND ruby # capture data after running the tests execute_process( COMMAND lcov --capture - --rc lcov_branch_coverage=1 - --rc genhtml_branch_coverage=1 + --rc 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