Skip to content

Commit

Permalink
Code review suggestions
Browse files Browse the repository at this point in the history
Signed-off-by: Gaurav Aggarwal <[email protected]>
  • Loading branch information
aggarg committed Jan 28, 2024
1 parent 3237918 commit 8a52e77
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 15 deletions.
24 changes: 17 additions & 7 deletions source/core_http_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ) );
}

Expand Down
16 changes: 9 additions & 7 deletions source/include/core_http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down
3 changes: 2 additions & 1 deletion source/include/core_http_client_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions test/unit-test/core_http_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down

0 comments on commit 8a52e77

Please sign in to comment.