From 8a52e77b84d2964bcb6b408775ce6b49991d33e4 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Sun, 28 Jan 2024 21:41:41 +0530 Subject: [PATCH] Code review suggestions Signed-off-by: Gaurav Aggarwal --- source/core_http_client.c | 24 ++++++++++++++++------- source/include/core_http_client.h | 16 ++++++++------- source/include/core_http_client_private.h | 3 ++- test/unit-test/core_http_utest.c | 4 ++++ 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 0b338eb6..ada77d2c 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -914,10 +914,9 @@ static int httpParserOnHeadersCompleteCallback( llhttp_t * pHttpParser ) /* If there is HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG opt-in we should stop * parsing here. */ - if( pResponse->respOptionFlags & HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG ) + if( ( pResponse->respOptionFlags & HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG ) != 0U ) { shouldContinueParse = LLHTTP_PAUSE_PARSING; - pResponse->pBody = ( const uint8_t * ) pParsingContext->pBufferCur; } return shouldContinueParse; @@ -1236,9 +1235,17 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, llhttp_resume_after_upgrade( &( pParsingContext->llhttpParser ) ); } - /* The next location to parse will always be after what has already - * been parsed. */ - pParsingContext->pBufferCur = parsingStartLoc + parseLen; + if( eReturn == HPE_PAUSED ) + { + /* The next location to parse is where the parser was paused. */ + pParsingContext->pBufferCur = pParsingContext->llhttpParser.error_pos; + } + else + { + /* The next location to parse is after what has already been parsed. */ + pParsingContext->pBufferCur = parsingStartLoc + parseLen; + } + returnStatus = processLlhttpError( &( pParsingContext->llhttpParser ) ); return returnStatus; @@ -2142,10 +2149,13 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t ( totalReceived < pResponse->bufferLen ) ) ? 1U : 0U; } - if( ( returnStatus == HTTPParserPaused ) && ( pResponse->respOptionFlags & HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG ) ) + if( ( returnStatus == HTTPParserPaused ) && + ( ( pResponse->respOptionFlags & HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG ) != 0U ) ) { returnStatus = HTTPSuccess; - /* There may be dangling data if we parse with do not parse body flag. To let libraries built on top of corehttp we expose it through body. */ + /* There may be dangling data if we parse with do not parse body flag. + * We expose this data through body to let the applications access it. */ + pResponse->pBody = ( const uint8_t * ) parsingContext.pBufferCur; pResponse->bodyLen = totalReceived - ( size_t ) ( ( ( uintptr_t ) pResponse->pBody ) - ( ( uintptr_t ) pResponse->pBuffer ) ); } diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index ebe92ca4..5239c3a1 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -156,7 +156,7 @@ /** * @defgroup http_response_option_flags HTTPResponse_t Flags * @brief Flags for #HTTPResponse_t.respOptionFlags. - * These flags control configurations of response parsing. + * These flags control the behavior of response parsing. * * Flags should be bitwise-ORed with each other to change the behavior of * #HTTPClient_ReceiveAndParseHttpResponse and #HTTPClient_Send. @@ -166,10 +166,11 @@ * @ingroup http_response_option_flags * @brief Set this flag to indicate that the response body should not be parsed. * - * Setting this will cause parser to stop after parsing the headers. Portion of raw body - * may be available in #HTTPResponse_t.pBody and #HTTPResponse_t.bodyLen. + * Setting this will cause parser to stop after parsing the headers. Portion of + * the raw body may be available in #HTTPResponse_t.pBody and + * #HTTPResponse_t.bodyLen. * - * This flag is valid only for #HTTPResponse_t respOptionFlags parameter. + * This flag is valid only for #HTTPResponse_t.respOptionFlags. */ #define HTTP_RESPONSE_DO_NOT_PARSE_BODY_FLAG 0x1U @@ -312,9 +313,10 @@ typedef enum HTTPStatus HTTPSecurityAlertInvalidContentLength, /** - * @brief Represents the state of the HTTP parser when it is paused. + * @brief Represents the paused state of the HTTP parser. * - * This state indicates that the parser has encountered a pause condition and is waiting for further input. + * This state indicates that the parser has encountered a pause condition + * and is waiting for further input. * * @see HTTPClient_Send * @see HTTPClient_ReceiveAndParseHttpResponse @@ -566,7 +568,7 @@ typedef struct HTTPResponse uint8_t areHeadersComplete; /** - * @brief Flags to activate other response configurations. + * @brief Flags to control the behavior of response parsing. * * Please see @ref http_response_option_flags for more information. */ diff --git a/source/include/core_http_client_private.h b/source/include/core_http_client_private.h index 72b21d6e..3ded00cc 100644 --- a/source/include/core_http_client_private.h +++ b/source/include/core_http_client_private.h @@ -167,7 +167,8 @@ #define LLHTTP_STOP_PARSING HPE_USER /** - * @brief Return value for llhttp registered callback to signal pause + * @brief Return value for llhttp registered callback to signal to pause + * further execution. */ #define LLHTTP_PAUSE_PARSING HPE_PAUSED diff --git a/test/unit-test/core_http_utest.c b/test/unit-test/core_http_utest.c index 04809c2f..ec22b1c2 100644 --- a/test/unit-test/core_http_utest.c +++ b/test/unit-test/core_http_utest.c @@ -1662,6 +1662,10 @@ void test_HTTPClient_strerror( void ) str = HTTPClient_strerror( status ); TEST_ASSERT_EQUAL_STRING( "HTTPSecurityAlertInvalidContentLength", str ); + status = HTTPParserPaused; + str = HTTPClient_strerror( status ); + TEST_ASSERT_EQUAL_STRING( "HTTPParserPaused", str ); + status = HTTPParserInternalError; str = HTTPClient_strerror( status ); TEST_ASSERT_EQUAL_STRING( "HTTPParserInternalError", str );