From 487aa9fa4e04f513ec248cb790688e542d2305b5 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Wed, 9 Dec 2020 17:46:54 -0800 Subject: [PATCH 01/18] Add definitions of timeout function and macros. --- source/include/core_http_client.h | 24 +++++++++++++ source/include/core_http_config_defaults.h | 39 ++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index f4b0c67f..4df6be9c 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -411,6 +411,15 @@ typedef struct HTTPClient_ResponseHeaderParsingCallback void * pContext; } HTTPClient_ResponseHeaderParsingCallback_t; +/** + * @ingroup http_callback_types + * @brief Application provided function to query the current time in + * milliseconds. + * + * @return The current time in milliseconds. + */ +typedef uint32_t (* HTTPClient_GetCurrentTimeFunc_t )( void ); + /** * @ingroup http_struct_types * @brief Represents an HTTP response. @@ -442,6 +451,21 @@ typedef struct HTTPResponse */ HTTPClient_ResponseHeaderParsingCallback_t * pHeaderParsingCallback; + /** + * @brief Optional callback for getting the system time. + * + * This is used to calculate the elapsed time between network sends and + * receives greater than zero. If this field is set to NULL, then network + * send and receive won't be retries after a zero is returned. + * + * If this function is set, then the maximum elapsed time between network + * receives greater than zero is set in HTTP_RECV_RETRY_TIMEOUT_MS. + * + * If this function is set, then the maximum elapsed time between network + * sends greater than zero is set in HTTP_SEND_RETRY_TIMEOUT_MS. + */ + HTTPClient_GetCurrentTimeFunc_t * pGetTimeFunction; + /** * @brief The starting location of the response headers in pBuffer. * diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index 578f87e6..938e1a54 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -64,6 +64,45 @@ #define HTTP_USER_AGENT_VALUE "my-platform-name" #endif +/** + * @brief The maximum duration between non-empty network reads while receiving + * an HTTP response via the #HTTPClient_Send API function. + * + * When an incoming HTTP response is detected, the transport receive function + * may be called multiple times until the end of the response is detected by the + * parser. This timeout represents the maximum duration that is allowed without + * any data reception from the network for the incoming response. + * + * If the timeout expires, the #HTTPClient_Send function will return + * #HTTPNetworkError. + * + * Possible values: Any positive 32 bit integer. A small timeout value + * is recommended.
+ * Default value: `10` + */ +#ifndef HTTP_RECV_RETRY_TIMEOUT_MS + #define HTTP_RECV_RETRY_TIMEOUT_MS ( 10U ) +#endif + +/** + * @brief The maximum duration between non-empty network transmissions while + * sending an HTTP request via the #HTTPClient_Send API function. + * + * When sending an HTTP request, the transport send function may be called multiple + * times until all of the required number of bytes are sent. + * This timeout represents the maximum duration that is allowed for no data + * transmission over the network through the transport send function. + * + * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. + * + * Possible values: Any positive 32 bit integer. A small timeout value + * is recommended.
+ * Default value: `10` + */ +#ifndef HTTP_SEND_RETRY_TIMEOUT_MS + #define HTTP_SEND_RETRY_TIMEOUT_MS ( 10U ) +#endif + /** * @brief Macro that is called in the HTTP Client library for logging "Error" level * messages. From 817ffd4472bb47b3327941ab7618dec20c9ad94d Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 00:25:04 -0800 Subject: [PATCH 02/18] Implement receive retry timeout. --- source/core_http_client.c | 192 +++++++++------------ source/include/core_http_client.h | 6 +- source/include/core_http_config_defaults.h | 22 +-- 3 files changed, 92 insertions(+), 128 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index c20abf0d..7175d44b 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -152,23 +152,6 @@ static HTTPStatus_t addRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, int32_t rangeStartOrlastNbytes, int32_t rangeEnd ); -/** - * @brief Receive HTTP response from the transport receive interface. - * - * @param[in] pTransport Transport interface. - * @param[in] pBuffer Response buffer. - * @param[in] bufferLen Length of the response buffer. - * @param[out] pBytesReceived Bytes received from the transport interface. - * - * @return Returns #HTTPSuccess if successful. If there was a network error or - * more bytes than what was specified were read, then #HTTPNetworkError is - * returned. - */ -static HTTPStatus_t receiveHttpData( const TransportInterface_t * pTransport, - uint8_t * pBuffer, - size_t bufferLen, - size_t * pBytesReceived ); - /** * @brief Get the status of the HTTP response given the parsing state and how * much data is in the response buffer. @@ -195,8 +178,9 @@ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, * @param[in] pResponse Response message to receive data from the network. * @param[in] pRequestHeaders Request headers for the corresponding HTTP request. * - * @return Returns #HTTPSuccess if successful. Please see #receiveHttpData, - * #parseHttpResponse, and #getFinalResponseStatus for other statuses returned. + * @return Returns #HTTPSuccess if successful. #HTTPNetworkError for a transport + * receive error. Please see #parseHttpResponse, and #getFinalResponseStatus for + * other statuses returned. */ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pTransport, HTTPResponse_t * pResponse, @@ -316,8 +300,10 @@ static int findHeaderOnHeaderCompleteCallback( http_parser * pHttpParser ); * server. * * @param[in] pParsingContext The parsing context to initialize. + * @param[in] pRequestHeaders Request headers for the corresponding HTTP request. */ -static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pParsingContext ); +static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pParsingContext, + const HTTPRequestHeaders_t * pRequestHeaders ); /** * @brief Parses the response buffer in @p pResponse. @@ -329,8 +315,6 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa * @param[in,out] pParsingContext The response parsing state. * @param[in,out] pResponse The response information to be updated. * @param[in] parseLen The next length to parse in pResponse->pBuffer. - * @param[in] isHeadResponse If the response is to a HEAD request this is set - * to 1, otherwise this is set to 0. * * @return One of the following: * - #HTTPSuccess @@ -339,8 +323,7 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa */ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, HTTPResponse_t * pResponse, - size_t parseLen, - uint8_t isHeadResponse ); + size_t parseLen ); /** * @brief Callback invoked during http_parser_execute() to indicate the start of @@ -926,13 +909,16 @@ static int httpParserOnMessageCompleteCallback( http_parser * pHttpParser ) /*-----------------------------------------------------------*/ -static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pParsingContext ) +static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pParsingContext, + const HTTPRequestHeaders_t * pRequestHeaders ) { assert( pParsingContext != NULL ); + assert( pRequestHeaders->headersLen >= HTTP_MINIMUM_REQUEST_LINE_LENGTH ); /* Initialize the third-party HTTP parser to parse responses. */ http_parser_init( &( pParsingContext->httpParser ), HTTP_RESPONSE ); + /* The parser will return an error if this header size limit is exceeded. */ http_parser_set_max_header_size( HTTP_MAX_RESPONSE_HEADERS_SIZE_BYTES ); /* No response has been parsed yet. */ @@ -940,6 +926,17 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa /* No response to update is associated with this parsing context yet. */ pParsingContext->pResponse = NULL; + + /* The parsing context needs to know if the response is for a HEAD request. + * The third-party parser requires parsing is manually indicated to stop + * in the httpParserOnHeadersCompleteCallback() for a HEAD response, + * otherwise the parser will not indicate the message was complete. */ + if( strncmp( ( const char * ) ( pRequestHeaders->pBuffer ), + HTTP_METHOD_HEAD, + sizeof( HTTP_METHOD_HEAD ) - 1U ) == 0 ) + { + pParsingContext->isHeadResponse = 1U; + } } /*-----------------------------------------------------------*/ @@ -1060,8 +1057,7 @@ static HTTPStatus_t processHttpParserError( const http_parser * pHttpParser ) static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, HTTPResponse_t * pResponse, - size_t parseLen, - uint8_t isHeadResponse ) + size_t parseLen ) { HTTPStatus_t returnStatus; http_parser_settings parserSettings = { 0 }; @@ -1073,7 +1069,6 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, assert( pParsingContext != NULL ); assert( pResponse != NULL ); - assert( isHeadResponse <= 1 ); /* If this is the first time this parsing context is used, then set the * response input. */ @@ -1081,8 +1076,6 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, { pParsingContext->pResponse = pResponse; pParsingContext->pBufferCur = ( const char * ) pResponse->pBuffer; - /* Set if this response is for a HEAD request. */ - pParsingContext->isHeadResponse = isHeadResponse; /* Initialize the status-code returned in the response. */ pResponse->statusCode = 0U; @@ -1848,58 +1841,6 @@ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, /*-----------------------------------------------------------*/ -static HTTPStatus_t receiveHttpData( const TransportInterface_t * pTransport, - uint8_t * pBuffer, - size_t bufferLen, - size_t * pBytesReceived ) -{ - HTTPStatus_t returnStatus = HTTPSuccess; - int32_t transportStatus = 0; - - assert( pTransport != NULL ); - assert( pTransport->recv != NULL ); - assert( pBuffer != NULL ); - assert( pBytesReceived != NULL ); - - transportStatus = pTransport->recv( pTransport->pNetworkContext, - pBuffer, - bufferLen ); - - /* A transport status of less than zero is an error. */ - if( transportStatus < 0 ) - { - LogError( ( "Failed to receive HTTP data: Transport recv() " - "returned error: TransportStatus=%ld", - ( long int ) transportStatus ) ); - returnStatus = HTTPNetworkError; - } - else if( transportStatus > 0 ) - { - /* It is a bug in the application's transport receive implementation if - * more bytes than expected are received. To avoid a possible overflow - * in converting bytesRemaining from unsigned to signed, this assert - * must exist after the check for transportStatus being negative. */ - assert( ( size_t ) transportStatus <= bufferLen ); - - /* Some or all of the specified data was received. */ - *pBytesReceived = ( size_t ) ( transportStatus ); - LogDebug( ( "Received data from the transport: BytesReceived=%ld", - ( long int ) transportStatus ) ); - } - else - { - /* When a zero is returned from the transport recv it will not be - * invoked again. */ - *pBytesReceived = 0U; - LogDebug( ( "Received zero bytes from transport recv(). Receiving " - "transport data is complete." ) ); - } - - return returnStatus; -} - -/*-----------------------------------------------------------*/ - static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, size_t totalReceived, size_t responseBufferLen ) @@ -1953,58 +1894,80 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT { HTTPStatus_t returnStatus = HTTPSuccess; size_t totalReceived = 0U; - size_t currentReceived = 0U; + int32_t currentReceived = 0U; HTTPParsingContext_t parsingContext = { 0 }; - uint8_t shouldRecv = 1U; - uint8_t isHeadResponse = 0U; + uint8_t shouldRecv = 1U, shouldParse = 1U; + uint32_t lastDataRecvTimeMs = 0U, timeSinceLastRecvMs = 0U; assert( pTransport != NULL ); assert( pTransport->recv != NULL ); assert( pResponse != NULL ); assert( pRequestHeaders != NULL ); - assert( pRequestHeaders->headersLen >= HTTP_MINIMUM_REQUEST_LINE_LENGTH ); - - /* The parsing context needs to know if the response is for a HEAD request. - * The third-party parser requires parsing is manually indicated to stop - * in the httpParserOnHeadersCompleteCallback() for a HEAD response, - * otherwise the parser will not indicate the message was complete. */ - if( strncmp( ( const char * ) ( pRequestHeaders->pBuffer ), - HTTP_METHOD_HEAD, - sizeof( HTTP_METHOD_HEAD ) - 1U ) == 0 ) - { - isHeadResponse = 1U; - } /* Initialize the parsing context for parsing the response received from the * network. */ - initializeParsingContextForFirstResponse( &parsingContext ); + initializeParsingContextForFirstResponse( &parsingContext, pRequestHeaders ); while( shouldRecv == 1U ) { /* Receive the HTTP response data into the pResponse->pBuffer. */ - returnStatus = receiveHttpData( pTransport, - pResponse->pBuffer + totalReceived, - pResponse->bufferLen - totalReceived, - ¤tReceived ); + currentReceived = pTransport->recv( pTransport->pNetworkContext, + pResponse->pBuffer + totalReceived, + pResponse->bufferLen - totalReceived ); - if( returnStatus == HTTPSuccess ) + /* Transport receive errors are negative. */ + if( currentReceived < 0 ) { - /* Data is received into the buffer and must be parsed. Parsing is - * invoked even with a length of zero. A length of zero indicates to - * the parser that there is no more data from the server (EOF). */ + LogError( ( "Failed to receive HTTP data: Transport recv() " + "returned error: TransportStatus=%ld", + ( long int ) transportStatus ) ); + returnStatus = HTTPNetworkError; + + /* Do not invoke the parser on network errors. */ + shouldParse = 0U; + } + else if( currentReceived > 0 ) + { + /* Reset the time of the last data received when data is received. */ + lastDataRecvTimeMs = pResponse->getTime(); + + /* Parsing is done on data as soon as it is received from the network. + * Because we cannot know how large the HTTP response will be in + * total, parsing will tell us if the end of the message is reached.*/ + shouldParse = 1U; + totalReceived += currentReceived; + } + else + { + timeSinceLastRecvMs = pResponse->getTime() - lastDataRecvTimeMs; + /* Do not invoke the response parsing for intermediate zero data. */ + shouldParse = 0U; + + /* Check if the allowed elapsed time between non-zero data has been + * reached. */ + if( timeSinceLastRecvMs >= HTTP_RECV_RETRY_TIMEOUT_MS ) + { + /* Invoke the parsing upon this final zero data to indicate + * to the parser that there is no more data available from the + * server. */ + shouldParse = 1U; + } + } + + if( shouldParse == 1U ) + { + /* Data is received into the buffer is immediately parsed. Parsing + * is invoked even with a length of zero. A length of zero indicates + * to the parser that there is no more data from the server (EOF). */ returnStatus = parseHttpResponse( &parsingContext, pResponse, - currentReceived, - isHeadResponse ); - totalReceived += currentReceived; + currentReceived ); } - /* Reading should continue if there are no errors in the transport recv - * or parsing, non-zero data was received from the network, - * the parser indicated the response message is not finished, and there - * is room in the response buffer. */ + /* Reading should continue if there are no errors in the transport receive + * or parsing, the parser indicated the response message is not finished, + * and there is room in the response buffer. */ shouldRecv = ( ( returnStatus == HTTPSuccess ) && - ( currentReceived > 0U ) && ( parsingContext.state != HTTP_PARSING_COMPLETE ) && ( totalReceived < pResponse->bufferLen ) ) ? 1U : 0U; } @@ -2022,6 +1985,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT return returnStatus; } + /*-----------------------------------------------------------*/ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index 4df6be9c..8f074058 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -453,18 +453,18 @@ typedef struct HTTPResponse /** * @brief Optional callback for getting the system time. - * + * * This is used to calculate the elapsed time between network sends and * receives greater than zero. If this field is set to NULL, then network * send and receive won't be retries after a zero is returned. * * If this function is set, then the maximum elapsed time between network * receives greater than zero is set in HTTP_RECV_RETRY_TIMEOUT_MS. - * + * * If this function is set, then the maximum elapsed time between network * sends greater than zero is set in HTTP_SEND_RETRY_TIMEOUT_MS. */ - HTTPClient_GetCurrentTimeFunc_t * pGetTimeFunction; + HTTPClient_GetCurrentTimeFunc_t getTime; /** * @brief The starting location of the response headers in pBuffer. diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index 938e1a54..ab3a8a19 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -67,21 +67,21 @@ /** * @brief The maximum duration between non-empty network reads while receiving * an HTTP response via the #HTTPClient_Send API function. - * - * When an incoming HTTP response is detected, the transport receive function - * may be called multiple times until the end of the response is detected by the - * parser. This timeout represents the maximum duration that is allowed without - * any data reception from the network for the incoming response. - * + * + * The transport receive function may be called multiple times until the end of + * the response is detected by the parser. This timeout represents the maximum + * duration that is allowed without any data reception from the network for the + * incoming response. + * * If the timeout expires, the #HTTPClient_Send function will return * #HTTPNetworkError. - * + * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
- * Default value: `10` + * Default value: `0` */ #ifndef HTTP_RECV_RETRY_TIMEOUT_MS - #define HTTP_RECV_RETRY_TIMEOUT_MS ( 10U ) + #define HTTP_RECV_RETRY_TIMEOUT_MS ( 0U ) #endif /** @@ -97,10 +97,10 @@ * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
- * Default value: `10` + * Default value: `0` */ #ifndef HTTP_SEND_RETRY_TIMEOUT_MS - #define HTTP_SEND_RETRY_TIMEOUT_MS ( 10U ) + #define HTTP_SEND_RETRY_TIMEOUT_MS ( 0U ) #endif /** From f862cde8b0a0a4dd603ac41227243fcf04af15e4 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 01:11:35 -0800 Subject: [PATCH 03/18] Implement sent retry timeout. --- source/core_http_client.c | 59 ++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 7175d44b..51ef8004 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -37,6 +37,7 @@ * @brief Send HTTP bytes over the transport send interface. * * @param[in] pTransport Transport interface. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. * @param[in] pData HTTP request data to send. * @param[in] dataLen HTTP request data length. * @@ -45,6 +46,7 @@ * returned. */ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, const uint8_t * pData, size_t dataLen ); @@ -52,6 +54,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, * @brief Send the HTTP headers over the transport send interface. * * @param[in] pTransport Transport interface. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. * @param[in] pRequestHeaders Request headers to send, it includes the buffer * and length. * @param[in] reqBodyLen The length of the request body to be sent. This is @@ -63,6 +66,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, * returned. */ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, HTTPRequestHeaders_t * pRequestHeaders, size_t reqBodyLen, uint32_t sendFlags ); @@ -84,6 +88,7 @@ static HTTPStatus_t addContentLengthHeader( HTTPRequestHeaders_t * pRequestHeade * @brief Send the HTTP body over the transport send interface. * * @param[in] pTransport Transport interface. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. * @param[in] pRequestBodyBuf Request body buffer. * @param[in] reqBodyBufLen Length of the request body buffer. * @@ -92,6 +97,7 @@ static HTTPStatus_t addContentLengthHeader( HTTPRequestHeaders_t * pRequestHeade * returned. */ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, const uint8_t * pRequestBodyBuf, size_t reqBodyBufLen ); @@ -1699,55 +1705,75 @@ HTTPStatus_t HTTPClient_AddRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, /*-----------------------------------------------------------*/ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, const uint8_t * pData, size_t dataLen ) { HTTPStatus_t returnStatus = HTTPSuccess; const uint8_t * pIndex = pData; - int32_t transportStatus = 0; + int32_t bytesSent = 0; size_t bytesRemaining = dataLen; + uint32_t lastSendTimeMs = 0U, timeSinceLastSendMs = 0U; assert( pTransport != NULL ); assert( pTransport->send != NULL ); assert( pData != NULL ); + /* Record the most recent time of successful transmission. */ + lastSendTimeMs = getTimestampMs(); + /* Loop until all data is sent. */ while( ( bytesRemaining > 0UL ) && ( returnStatus != HTTPNetworkError ) ) { - transportStatus = pTransport->send( pTransport->pNetworkContext, + bytesSent = pTransport->send( pTransport->pNetworkContext, pIndex, bytesRemaining ); /* A transport status of less than zero is an error. */ - if( transportStatus < 0 ) + if( bytesSent < 0 ) { LogError( ( "Failed to send HTTP data: Transport send()" " returned error: TransportStatus=%ld", - ( long int ) transportStatus ) ); + ( long int ) bytesSent ) ); returnStatus = HTTPNetworkError; } - else + else if( bytesSent > 0 ) { /* It is a bug in the application's transport send implementation if * more bytes than expected are sent. To avoid a possible overflow * in converting bytesRemaining from unsigned to signed, this assert * must exist after the check for transportStatus being negative. */ - assert( ( size_t ) transportStatus <= bytesRemaining ); + assert( ( size_t ) bytesSent <= bytesRemaining ); - bytesRemaining -= ( size_t ) transportStatus; - pIndex += transportStatus; + /* Record the most recent time of successful transmission. */ + lastSendTimeMs = getTimestampMs(); + + bytesRemaining -= ( size_t ) bytesSent; + pIndex += bytesSent; LogDebug( ( "Sent HTTP data over the transport: " "BytesSent=%ld, BytesRemaining=%lu, TotalBytesSent=%lu", - ( long int ) transportStatus, + ( long int ) bytesSent, ( unsigned long ) bytesRemaining, ( unsigned long ) ( dataLen - bytesRemaining ) ) ); } + else + { + /* No bytes were sent over the network. */ + timeSinceLastSendMs = calculateElapsedTime( getTimestampMs(), lastSendTimeMs ); + /* Check for timeout if we have been waiting to send any data over + * the network. */ + if( timeSinceLastSendMs >= HTTP_SEND_RETRY_TIMEOUT_MS ) + { + LogError( ( "Unable to send packet: Timed out in transport send." ) ); + returnStatus = HTTPNetworkError; + } + } } - if( returnStatus == HTTPSuccess ) + if( bytesRemaining < dataLen ) { LogDebug( ( "Sent HTTP data over the transport: BytesSent=%ld", - ( long int ) transportStatus ) ); + ( unsigned long ) ( dataLen - bytesRemaining ) ) ); } return returnStatus; @@ -1788,6 +1814,7 @@ static HTTPStatus_t addContentLengthHeader( HTTPRequestHeaders_t * pRequestHeade /*-----------------------------------------------------------*/ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, HTTPRequestHeaders_t * pRequestHeaders, size_t reqBodyLen, uint32_t sendFlags ) @@ -1813,7 +1840,10 @@ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, { LogDebug( ( "Sending HTTP request headers: HeaderBytes=%lu", ( unsigned long ) ( pRequestHeaders->headersLen ) ) ); - returnStatus = sendHttpData( pTransport, pRequestHeaders->pBuffer, pRequestHeaders->headersLen ); + returnStatus = sendHttpData( pTransport, + getTimestampMs, + pRequestHeaders->pBuffer, + pRequestHeaders->headersLen ); } return returnStatus; @@ -1822,6 +1852,7 @@ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, /*-----------------------------------------------------------*/ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, const uint8_t * pRequestBodyBuf, size_t reqBodyBufLen ) { @@ -1834,7 +1865,7 @@ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, /* Send the request body. */ LogDebug( ( "Sending the HTTP request body: BodyBytes=%lu", ( unsigned long ) reqBodyBufLen ) ); - returnStatus = sendHttpData( pTransport, pRequestBodyBuf, reqBodyBufLen ); + returnStatus = sendHttpData( pTransport, getTimestampMs, pRequestBodyBuf, reqBodyBufLen ); return returnStatus; } @@ -2066,6 +2097,7 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, if( returnStatus == HTTPSuccess ) { returnStatus = sendHttpHeaders( pTransport, + pResponse->getTime, pRequestHeaders, reqBodyBufLen, sendFlags ); @@ -2077,6 +2109,7 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, if( pRequestBodyBuf != NULL ) { returnStatus = sendHttpBody( pTransport, + pResponse->getTime, pRequestBodyBuf, reqBodyBufLen ); } From accf9446ef23a13ae7781cfac209b7b10bfcc25c Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 01:31:39 -0800 Subject: [PATCH 04/18] Handle the case where the user did not set the getTime function. --- source/core_http_client.c | 51 +++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 51ef8004..0a4a88ff 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -33,6 +33,14 @@ /*-----------------------------------------------------------*/ +/** + * @brief When #HTTPResponse_t.getTime is set to NULL in #HTTPClient_Send then + * this function will replace that field. + * + * @return This function always returns zero. + */ +static uint32_t getZeroTimestampMs( void ); + /** * @brief Send HTTP bytes over the transport send interface. * @@ -518,6 +526,13 @@ static HTTPStatus_t processHttpParserError( const http_parser * pHttpParser ); /*-----------------------------------------------------------*/ +static uint32_t getZeroTimestampMs( void ) +{ + return 0U; +} + +/*-----------------------------------------------------------*/ + static void processCompleteHeader( HTTPParsingContext_t * pParsingContext ) { HTTPResponse_t * pResponse = NULL; @@ -1714,11 +1729,19 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, int32_t bytesSent = 0; size_t bytesRemaining = dataLen; uint32_t lastSendTimeMs = 0U, timeSinceLastSendMs = 0U; + uint32_t retryTimeoutMs = HTTP_SEND_RETRY_TIMEOUT_MS; assert( pTransport != NULL ); assert( pTransport->send != NULL ); assert( pData != NULL ); + /* If the timestamp function was undefined by the application, then do not + * retry the transport send. */ + if( getTimestampMs == getZeroTimestampMs ) + { + retryTimeoutMs = 0U; + } + /* Record the most recent time of successful transmission. */ lastSendTimeMs = getTimestampMs(); @@ -1762,7 +1785,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, timeSinceLastSendMs = calculateElapsedTime( getTimestampMs(), lastSendTimeMs ); /* Check for timeout if we have been waiting to send any data over * the network. */ - if( timeSinceLastSendMs >= HTTP_SEND_RETRY_TIMEOUT_MS ) + if( timeSinceLastSendMs >= retryTimeoutMs ) { LogError( ( "Unable to send packet: Timed out in transport send." ) ); returnStatus = HTTPNetworkError; @@ -1928,7 +1951,8 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT int32_t currentReceived = 0U; HTTPParsingContext_t parsingContext = { 0 }; uint8_t shouldRecv = 1U, shouldParse = 1U; - uint32_t lastDataRecvTimeMs = 0U, timeSinceLastRecvMs = 0U; + uint32_t lastRecvTimeMs = 0U, timeSinceLastRecvMs = 0U; + uint32_t retryTimeoutMs = HTTP_RECV_RETRY_TIMEOUT_MS; assert( pTransport != NULL ); assert( pTransport->recv != NULL ); @@ -1939,6 +1963,17 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT * network. */ initializeParsingContextForFirstResponse( &parsingContext, pRequestHeaders ); + /* If the timestamp function was undefined by the application, then do not + * retry the transport receive. */ + if( pResponse->getTime == getZeroTimestampMs ) + { + retryTimeoutMs = 0U; + } + + /* Start the last receive time to allow retries on the first zero data + * receive. */ + lastRecvTimeMs = pResponse->getTime(); + while( shouldRecv == 1U ) { /* Receive the HTTP response data into the pResponse->pBuffer. */ @@ -1960,7 +1995,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT else if( currentReceived > 0 ) { /* Reset the time of the last data received when data is received. */ - lastDataRecvTimeMs = pResponse->getTime(); + lastRecvTimeMs = pResponse->getTime(); /* Parsing is done on data as soon as it is received from the network. * Because we cannot know how large the HTTP response will be in @@ -1970,13 +2005,13 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT } else { - timeSinceLastRecvMs = pResponse->getTime() - lastDataRecvTimeMs; + timeSinceLastRecvMs = pResponse->getTime() - lastRecvTimeMs; /* Do not invoke the response parsing for intermediate zero data. */ shouldParse = 0U; /* Check if the allowed elapsed time between non-zero data has been * reached. */ - if( timeSinceLastRecvMs >= HTTP_RECV_RETRY_TIMEOUT_MS ) + if( timeSinceLastRecvMs >= retryTimeoutMs ) { /* Invoke the parsing upon this final zero data to indicate * to the parser that there is no more data available from the @@ -2088,6 +2123,12 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, ( unsigned long ) reqBodyBufLen ) ); returnStatus = HTTPInvalidParameter; } + else if( pResponse->getTime == NULL ) + { + /* Set a dummy timestamp function when the application did not configure + * one. */ + pResponse->getTime = getZeroTimestampMs; + } else { /* Empty else for MISRA 15.7 compliance. */ From 541b04af490db420346a3c20b873541a1dd50359 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 01:51:53 -0800 Subject: [PATCH 05/18] Fix documentation, complexity, transport interface, and spelling. --- lexicon.txt | 7 ++ source/core_http_client.c | 107 +++++++++++++++++-------- source/interface/transport_interface.h | 63 ++++++++++++--- 3 files changed, 132 insertions(+), 45 deletions(-) diff --git a/lexicon.txt b/lexicon.txt index cf5741e0..e469b264 100644 --- a/lexicon.txt +++ b/lexicon.txt @@ -9,6 +9,7 @@ api apis ascii aws +bool br bufferlen bufferlength @@ -17,6 +18,7 @@ bytesremaining bytessent bytestorecv bytestosend +calltlsrecvfunc cb cbmc chk @@ -52,6 +54,8 @@ findheaderonheadercompletecallback findheadervalueparsercallback firstpartbytes getfinalresponsestatus +gettime +gettimestampms github headercount headerslen @@ -211,6 +215,8 @@ rfc sdk senderrorcall sendflags +sendhttpbody +sendhttpheaders sendpartialcall sizeof snprintf @@ -225,6 +231,7 @@ tcpsocketcontext tls tlscontext tlsrecv +tlsrecvcount tlssend toascii totalreceived diff --git a/source/core_http_client.c b/source/core_http_client.c index 0a4a88ff..ddeb41bf 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -36,7 +36,7 @@ /** * @brief When #HTTPResponse_t.getTime is set to NULL in #HTTPClient_Send then * this function will replace that field. - * + * * @return This function always returns zero. */ static uint32_t getZeroTimestampMs( void ); @@ -200,6 +200,26 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT HTTPResponse_t * pResponse, const HTTPRequestHeaders_t * pRequestHeaders ); +/** + * @brief Send the HTTP request over the network. + * + * @param[in] pTransport Transport interface. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. + * @param[in] pRequestHeaders Request headers to send over the network. + * @param[in] pRequestBodyBuf Request body buffer to send over the network. + * @param[in] reqBodyBufLen Length of the request body buffer. + * @param[in] sendFlags Application provided flags to #HTTPClient_Send. + * + * @return Returns #HTTPSuccess if successful. Please see #sendHttpHeaders and + * #sendHttpBody for other statuses returned. + */ +static HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + const uint8_t * pRequestBodyBuf, + size_t reqBodyBufLen, + uint32_t sendFlags ); + /** * @brief Converts an integer value to its ASCII representation in the passed * buffer. @@ -1749,15 +1769,14 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, while( ( bytesRemaining > 0UL ) && ( returnStatus != HTTPNetworkError ) ) { bytesSent = pTransport->send( pTransport->pNetworkContext, - pIndex, - bytesRemaining ); + pIndex, + bytesRemaining ); /* A transport status of less than zero is an error. */ if( bytesSent < 0 ) { - LogError( ( "Failed to send HTTP data: Transport send()" - " returned error: TransportStatus=%ld", - ( long int ) bytesSent ) ); + LogError( ( "Failed to send data: Transport send error: " + "TransportStatus=%ld", ( long int ) bytesSent ) ); returnStatus = HTTPNetworkError; } else if( bytesSent > 0 ) @@ -1773,7 +1792,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, bytesRemaining -= ( size_t ) bytesSent; pIndex += bytesSent; - LogDebug( ( "Sent HTTP data over the transport: " + LogDebug( ( "Sent data over the transport: " "BytesSent=%ld, BytesRemaining=%lu, TotalBytesSent=%lu", ( long int ) bytesSent, ( unsigned long ) bytesRemaining, @@ -1782,7 +1801,8 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, else { /* No bytes were sent over the network. */ - timeSinceLastSendMs = calculateElapsedTime( getTimestampMs(), lastSendTimeMs ); + timeSinceLastSendMs = getTimestampMs() - lastSendTimeMs; + /* Check for timeout if we have been waiting to send any data over * the network. */ if( timeSinceLastSendMs >= retryTimeoutMs ) @@ -1793,12 +1813,6 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, } } - if( bytesRemaining < dataLen ) - { - LogDebug( ( "Sent HTTP data over the transport: BytesSent=%ld", - ( unsigned long ) ( dataLen - bytesRemaining ) ) ); - } - return returnStatus; } @@ -1865,7 +1879,7 @@ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, ( unsigned long ) ( pRequestHeaders->headersLen ) ) ); returnStatus = sendHttpData( pTransport, getTimestampMs, - pRequestHeaders->pBuffer, + pRequestHeaders->pBuffer, pRequestHeaders->headersLen ); } @@ -2051,6 +2065,47 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT return returnStatus; } +/*-----------------------------------------------------------*/ + +HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + const uint8_t * pRequestBodyBuf, + size_t reqBodyBufLen, + uint32_t sendFlags ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + assert( pTransport != NULL ); + assert( pRequestHeaders != NULL ); + assert( pRequestBodyBuf != NULL ); + assert( getTimestampMs != NULL ); + + /* Send the headers, which are at one location in memory. */ + returnStatus = sendHttpHeaders( pTransport, + getTimestampMs, + pRequestHeaders, + reqBodyBufLen, + sendFlags ); + + /* Send the body, which is at another location in memory. */ + if( returnStatus == HTTPSuccess ) + { + if( pRequestBodyBuf != NULL ) + { + returnStatus = sendHttpBody( pTransport, + getTimestampMs, + pRequestBodyBuf, + reqBodyBufLen ); + } + else + { + LogDebug( ( "A request body was not sent: pRequestBodyBuf is NULL." ) ); + } + } + + return returnStatus; +} /*-----------------------------------------------------------*/ @@ -2125,7 +2180,7 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, } else if( pResponse->getTime == NULL ) { - /* Set a dummy timestamp function when the application did not configure + /* Set a zero timestamp function when the application did not configure * one. */ pResponse->getTime = getZeroTimestampMs; } @@ -2134,32 +2189,16 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, /* Empty else for MISRA 15.7 compliance. */ } - /* Send the headers, which are at one location in memory. */ if( returnStatus == HTTPSuccess ) { - returnStatus = sendHttpHeaders( pTransport, + returnStatus = sendHttpRequest( pTransport, pResponse->getTime, pRequestHeaders, + pRequestBodyBuf, reqBodyBufLen, sendFlags ); } - /* Send the body, which is at another location in memory. */ - if( returnStatus == HTTPSuccess ) - { - if( pRequestBodyBuf != NULL ) - { - returnStatus = sendHttpBody( pTransport, - pResponse->getTime, - pRequestBodyBuf, - reqBodyBufLen ); - } - else - { - LogDebug( ( "A request body was not sent: pRequestBodyBuf is NULL." ) ); - } - } - if( returnStatus == HTTPSuccess ) { /* If the application chooses to receive a response, then pResponse diff --git a/source/interface/transport_interface.h b/source/interface/transport_interface.h index e166bf1a..ecfe2af0 100644 --- a/source/interface/transport_interface.h +++ b/source/interface/transport_interface.h @@ -96,16 +96,37 @@ * size_t bytesToRecv ) * { * int32_t bytesReceived = 0; - * bytesReceived = TLSRecv( pNetworkContext->tlsContext, - * pBuffer, - * bytesToRecv, - * MY_SOCKET_TIMEOUT ); - * if( bytesReceived < 0 ) + * bool callTlsRecvFunc = true; + * + * // For a single byte read request, check if data is available on the network. + * if( bytesToRecv == 1 ) * { - * // Handle socket error. + * // If no data is available on the network, do not call TLSRecv + * // to avoid blocking for socket timeout. + * if( TLSRecvCount( pNetworkContext->tlsContext ) == 0 ) + * { + * callTlsRecvFunc = false; + * } * } - * // Handle other cases. * + * if( callTlsRecvFunc == true ) + * { + * bytesReceived = TLSRecv( pNetworkContext->tlsContext, + * pBuffer, + * bytesToRecv, + * MY_SOCKET_TIMEOUT ); + * if( bytesReceived < 0 ) + * { + * // If the error code represents a timeout, then the return + * // code should be translated to zero so that the caller + * // can retry the read operation. + * if( bytesReceived == MY_SOCKET_ERROR_TIMEOUT ) + * { + * bytesReceived = 0; + * } + * } + * // Handle other cases. + * } * return bytesReceived; * } * @endcode @@ -132,7 +153,14 @@ * pBuffer, * bytesToSend, * MY_SOCKET_TIMEOUT ); - * if( bytesSent < 0 ) + * + * // If underlying TCP buffer is full, set the return value to zero + * // so that caller can retry the send operation. + * if( bytesSent == MY_SOCKET_ERROR_BUFFER_FULL ) + * { + * bytesSent = 0; + * } + * else if( bytesSent < 0 ) * { * // Handle socket error. * } @@ -159,6 +187,15 @@ typedef struct NetworkContext NetworkContext_t; * @transportcallback * @brief Transport interface for receiving data on the network. * + * @note It is RECOMMENDED that the transport receive implementation + * does NOT block when requested to read a single byte. A single byte + * read request can be made by the caller to check whether there is a + * new frame available on the network for reading. + * However, the receive implementation MAY block for a timeout period when + * it is requested to read more than 1 byte. This is because once the caller + * is aware that a new frame is available to read on the network, then + * the likelihood of reading more than one byte over the network becomes high. + * * @param[in] pNetworkContext Implementation-defined network context. * @param[in] pBuffer Buffer to receive the data into. * @param[in] bytesToRecv Number of bytes requested from the network. @@ -167,8 +204,10 @@ typedef struct NetworkContext NetworkContext_t; * error. * * @note If no data is available on the network to read and no error - * has occurred, zero MUST be the return value. Zero MUST NOT be used - * if a network disconnection has occurred. + * has occurred, zero MUST be the return value. A zero return value + * SHOULD represent that the read operation can be retried by calling + * the API function. Zero MUST NOT be returned if a network disconnection + * has occurred. */ /* @[define_transportrecv] */ typedef int32_t ( * TransportRecv_t )( NetworkContext_t * pNetworkContext, @@ -188,7 +227,9 @@ typedef int32_t ( * TransportRecv_t )( NetworkContext_t * pNetworkContext, * * @note If no data is transmitted over the network due to a full TX buffer and * no network error has occurred, this MUST return zero as the return value. - * Zero MUST NOT be returned if a network disconnection has occurred. + * A zero return value SHOULD represent that the send operation can be retried + * by calling the API function. Zero MUST NOT be returned if a network disconnection + * has occurred. */ /* @[define_transportsend] */ typedef int32_t ( * TransportSend_t )( NetworkContext_t * pNetworkContext, From 3d15e45a55d308691dbb1dc95421172607d55bae Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 15:37:14 -0800 Subject: [PATCH 06/18] Fix errors to make all current tests pass. --- source/core_http_client.c | 52 +++++++++++---------------- test/unit-test/core_http_send_utest.c | 2 +- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index ddeb41bf..599939a2 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -1964,7 +1964,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT size_t totalReceived = 0U; int32_t currentReceived = 0U; HTTPParsingContext_t parsingContext = { 0 }; - uint8_t shouldRecv = 1U, shouldParse = 1U; + uint8_t shouldRecv = 1U, shouldParse = 1U, timeoutReached = 0U; uint32_t lastRecvTimeMs = 0U, timeSinceLastRecvMs = 0U; uint32_t retryTimeoutMs = HTTP_RECV_RETRY_TIMEOUT_MS; @@ -2031,6 +2031,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT * to the parser that there is no more data available from the * server. */ shouldParse = 1U; + timeoutReached = 1U; } } @@ -2045,9 +2046,11 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT } /* Reading should continue if there are no errors in the transport receive - * or parsing, the parser indicated the response message is not finished, - * and there is room in the response buffer. */ + * or parsing, the retry on zero data timeout has not been reached, the + * parser indicated the response message is not finished, and there is + * room in the response buffer. */ shouldRecv = ( ( returnStatus == HTTPSuccess ) && + ( timeoutReached == 0U ) && ( parsingContext.state != HTTP_PARSING_COMPLETE ) && ( totalReceived < pResponse->bufferLen ) ) ? 1U : 0U; } @@ -2078,7 +2081,8 @@ HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, assert( pTransport != NULL ); assert( pRequestHeaders != NULL ); - assert( pRequestBodyBuf != NULL ); + assert( ( pRequestBodyBuf != NULL ) || + ( ( pRequestBodyBuf == NULL ) && ( reqBodyBufLen == 0 ) ) ); assert( getTimestampMs != NULL ); /* Send the headers, which are at one location in memory. */ @@ -2116,32 +2120,27 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, HTTPResponse_t * pResponse, uint32_t sendFlags ) { - HTTPStatus_t returnStatus = HTTPSuccess; + HTTPStatus_t returnStatus = HTTPInvalidParameter; if( pTransport == NULL ) { LogError( ( "Parameter check failed: pTransport interface is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( pTransport->send == NULL ) { LogError( ( "Parameter check failed: pTransport->send is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( pTransport->recv == NULL ) { LogError( ( "Parameter check failed: pTransport->recv is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( pRequestHeaders == NULL ) { LogError( ( "Parameter check failed: pRequestHeaders is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( pRequestHeaders->pBuffer == NULL ) { LogError( ( "Parameter check failed: pRequestHeaders->pBuffer is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( pRequestHeaders->headersLen < HTTP_MINIMUM_REQUEST_LINE_LENGTH ) { @@ -2150,24 +2149,26 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, "MinimumRequiredLength=%u, HeadersLength=%lu", HTTP_MINIMUM_REQUEST_LINE_LENGTH, ( unsigned long ) ( pRequestHeaders->headersLen ) ) ); - returnStatus = HTTPInvalidParameter; } else if( pRequestHeaders->headersLen > pRequestHeaders->bufferLen ) { LogError( ( "Parameter check failed: pRequestHeaders->headersLen > " "pRequestHeaders->bufferLen." ) ); - returnStatus = HTTPInvalidParameter; } - else if( ( pResponse != NULL ) && ( pResponse->pBuffer == NULL ) ) + else if( pResponse == NULL ) + { + LogError( ( "Parameter check failed: pResponse is NULL. " ) ); + } + else if( pResponse->pBuffer == NULL ) { LogError( ( "Parameter check failed: pResponse->pBuffer is NULL." ) ); - returnStatus = HTTPInvalidParameter; } else if( ( pRequestBodyBuf == NULL ) && ( reqBodyBufLen > 0U ) ) { + /* If there is no body to send we must ensure that the reqBodyBufLen is + * zero so that no Content-Length header is automatically written. */ LogError( ( "Parameter check failed: pRequestBodyBuf is NULL, but " "reqBodyBufLen is greater than zero." ) ); - returnStatus = HTTPInvalidParameter; } else if( reqBodyBufLen > ( size_t ) ( INT32_MAX ) ) { @@ -2176,17 +2177,13 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, LogError( ( "Parameter check failed: reqBodyBufLen > INT32_MAX." "reqBodyBufLen=%lu", ( unsigned long ) reqBodyBufLen ) ); - returnStatus = HTTPInvalidParameter; } - else if( pResponse->getTime == NULL ) + else { /* Set a zero timestamp function when the application did not configure * one. */ pResponse->getTime = getZeroTimestampMs; - } - else - { - /* Empty else for MISRA 15.7 compliance. */ + returnStatus = HTTPSuccess; } if( returnStatus == HTTPSuccess ) @@ -2203,16 +2200,9 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, { /* If the application chooses to receive a response, then pResponse * will not be NULL. */ - if( pResponse != NULL ) - { - returnStatus = receiveAndParseHttpResponse( pTransport, - pResponse, - pRequestHeaders ); - } - else - { - LogDebug( ( "Response ignored: pResponse is NULL." ) ); - } + returnStatus = receiveAndParseHttpResponse( pTransport, + pResponse, + pRequestHeaders ); } return returnStatus; diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index a90d9b3a..d66dd208 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -1122,7 +1122,7 @@ void test_HTTPClient_Send_null_response( void ) HTTP_TEST_REQUEST_PUT_BODY_LENGTH, NULL, 0U ); - TEST_ASSERT_EQUAL( HTTPSuccess, returnStatus ); + TEST_ASSERT_EQUAL( HTTPInvalidParameter, returnStatus ); } /*-----------------------------------------------------------*/ From f314b60561cd71b4b9d49c15c135184347f552af Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 17:30:38 -0800 Subject: [PATCH 07/18] Add test for coverage of a receive timeout greater than zero. --- source/core_http_client.c | 14 +++-- test/unit-test/core_http_config.h | 27 ++++++++- test/unit-test/core_http_send_utest.c | 85 +++++++++++++++++++++------ 3 files changed, 101 insertions(+), 25 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 599939a2..2932a09b 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -1762,7 +1762,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, retryTimeoutMs = 0U; } - /* Record the most recent time of successful transmission. */ + /* Start the last send time to allow retries on the first zero data sent. */ lastSendTimeMs = getTimestampMs(); /* Loop until all data is sent. */ @@ -2180,9 +2180,13 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, } else { - /* Set a zero timestamp function when the application did not configure - * one. */ - pResponse->getTime = getZeroTimestampMs; + if( pResponse->getTime == NULL ) + { + /* Set a zero timestamp function when the application did not configure + * one. */ + pResponse->getTime = getZeroTimestampMs; + } + returnStatus = HTTPSuccess; } @@ -2198,8 +2202,6 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, if( returnStatus == HTTPSuccess ) { - /* If the application chooses to receive a response, then pResponse - * will not be NULL. */ returnStatus = receiveAndParseHttpResponse( pTransport, pResponse, pRequestHeaders ); diff --git a/test/unit-test/core_http_config.h b/test/unit-test/core_http_config.h index c045c844..b6cd2b7c 100644 --- a/test/unit-test/core_http_config.h +++ b/test/unit-test/core_http_config.h @@ -23,6 +23,31 @@ #ifndef CORE_HTTP_CONFIG_H__ #define CORE_HTTP_CONFIG_H__ -/* Dummy core_http_config.h for building the library. */ +/** + * @brief The maximum duration between non-empty network reads while receiving + * an HTTP response via the #HTTPClient_Send API function. + * + * The transport receive function may be called multiple times until the end of + * the response is detected by the parser. This timeout represents the maximum + * duration that is allowed without any data reception from the network for the + * incoming response. + * + * If the timeout expires, the #HTTPClient_Send function will return + * #HTTPNetworkError. + */ +#define HTTP_RECV_RETRY_TIMEOUT_MS ( 3U ) + +/** + * @brief The maximum duration between non-empty network transmissions while + * sending an HTTP request via the #HTTPClient_Send API function. + * + * When sending an HTTP request, the transport send function may be called multiple + * times until all of the required number of bytes are sent. + * This timeout represents the maximum duration that is allowed for no data + * transmission over the network through the transport send function. + * + * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. + */ +#define HTTP_SEND_RETRY_TIMEOUT_MS ( 3U ) #endif /* ifndef CORE_HTTP_CONFIG_H__ */ diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index d66dd208..c1f4afa5 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -212,7 +212,7 @@ static uint8_t recvCurrentCall = 0; /* The test sets this variable to indicate which call count count of transport * receive to return an error from. */ -static uint8_t recvStopCall = 0; +static uint8_t recvTimeoutCall = 0; /* The count of times a mocked http_parser_execute callback has been invoked. */ static uint8_t httpParserExecuteCallCount; @@ -229,6 +229,14 @@ static HTTPRequestHeaders_t requestHeaders = { 0 }; /* Header parsing callback shared among the tests. */ static HTTPClient_ResponseHeaderParsingCallback_t headerParsingCallback = { 0 }; +/* A mocked timer query function that increments on every call. */ +static uint32_t getTestTime( void ) +{ + static entryTime = 0; + + return entryTime++; +} + /* Application callback for intercepting the headers during the parse of a new * response from the mocked network interface. */ static void onHeaderCallback( void * pContext, @@ -267,9 +275,11 @@ static int32_t transportSendSuccess( NetworkContext_t * pNetworkContext, { ( void ) pNetworkContext; + sendCurrentCall++; + if( checkContentLength == 1U ) { - if( sendCurrentCall == 0U ) + if( sendCurrentCall == 1U ) { size_t contentLengthAndHeaderEndLen = HTTP_TEST_REQUEST_PUT_CONTENT_LENGTH_EXPECTED_LENGTH; char * pContentLengthStart = ( ( ( char * ) pBuffer ) + bytesToWrite ) - contentLengthAndHeaderEndLen; @@ -280,7 +290,6 @@ static int32_t transportSendSuccess( NetworkContext_t * pNetworkContext, } } - sendCurrentCall++; return bytesToWrite; } @@ -295,12 +304,13 @@ static int32_t transportSendNetworkError( NetworkContext_t * pNetworkContext, ( void ) pBuffer; int32_t retVal = bytesToWrite; + sendCurrentCall++; + if( sendErrorCall == sendCurrentCall ) { retVal = -1; } - sendCurrentCall++; return retVal; } @@ -316,12 +326,13 @@ static int32_t transportSendLessThanBytesToWrite( NetworkContext_t * pNetworkCon ( void ) pBuffer; int32_t retVal = bytesToWrite; + sendCurrentCall++; + if( sendPartialCall == sendCurrentCall ) { retVal -= 1; } - sendCurrentCall++; return retVal; } @@ -329,7 +340,7 @@ static int32_t transportSendLessThanBytesToWrite( NetworkContext_t * pNetworkCon * firstPartBytes on the first call, then sends the rest of the response in the * second call. The response to send is set in pNetworkData and the current * call count is kept track of in recvCurrentCall. This function will return - * zero (timeout condition) when recvStopCall matches recvCurrentCall. */ + * zero (timeout condition) when recvTimeoutCall matches recvCurrentCall. */ static int32_t transportRecvSuccess( NetworkContext_t * pNetworkContext, void * pBuffer, size_t bytesToRead ) @@ -337,15 +348,18 @@ static int32_t transportRecvSuccess( NetworkContext_t * pNetworkContext, ( void ) pNetworkContext; size_t bytesToCopy = 0; + /* Increment this call count. */ + recvCurrentCall++; + /* To test stopping in the middle of a response message, check that the * flags are set. */ - if( recvStopCall == recvCurrentCall ) + if( recvTimeoutCall == recvCurrentCall ) { return 0; } /* If this is the first call, then copy the specific first bytes. */ - if( recvCurrentCall == 0 ) + if( recvCurrentCall == 1 ) { bytesToCopy = firstPartBytes; } @@ -363,7 +377,6 @@ static int32_t transportRecvSuccess( NetworkContext_t * pNetworkContext, memcpy( pBuffer, pNetworkData, bytesToCopy ); pNetworkData += bytesToCopy; networkDataLen -= bytesToCopy; - recvCurrentCall++; return bytesToCopy; } @@ -738,7 +751,7 @@ void setUp( void ) networkDataLen = HTTP_TEST_RESPONSE_HEAD_LENGTH; firstPartBytes = networkDataLen; recvCurrentCall = 0; - recvStopCall = UINT8_MAX; + recvTimeoutCall = UINT8_MAX; httpParserExecuteCallCount = 0; httpParsingErrno = HPE_OK; transportInterface.recv = transportRecvSuccess; @@ -1041,7 +1054,8 @@ void test_HTTPClient_Send_timeout_recv_immediate( void ) http_parser_execute_ExpectAnyArgsAndReturn( 0 ); - recvStopCall = 0; + /* Return a zero on the first call. */ + recvTimeoutCall = 1; returnStatus = HTTPClient_Send( &transportInterface, &requestHeaders, NULL, @@ -1063,7 +1077,8 @@ void test_HTTPClient_Send_timeout_partial_response( void ) http_errno_description_IgnoreAndReturn( "Dummy unit test print." ); firstPartBytes = HTTP_TEST_RESPONSE_HEAD_PARTIAL_HEADER_VALUE_LENGTH; - recvStopCall = 1; + /* Return a zero on the second transport receive call. */ + recvTimeoutCall = 2; returnStatus = HTTPClient_Send( &transportInterface, &requestHeaders, @@ -1076,6 +1091,36 @@ void test_HTTPClient_Send_timeout_partial_response( void ) /*-----------------------------------------------------------*/ +/* Test a timeout is received from the first network read, but the read after + * the response is received fully. */ +void test_HTTPClient_Send_timeout_recv_immediately_but_success_after( void ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + http_parser_execute_Stub( http_parser_execute_whole_response ); + http_errno_description_IgnoreAndReturn( "Dummy unit test print." ); + + /* Set the optional time keeping function to retry the receive when zero + * data is read from the network. */ + response.getTime = getTestTime; + /* On the first call to the transport recieve, return a zero. */ + recvTimeoutCall = 1; + + /* With HTTP_RECV_RETRY_TIMEOUT_MS set to greater than 1U in core_http_config.h + * we ensure that the retry timeout is not reached and the transport receive + * is called again. */ + + returnStatus = HTTPClient_Send( &transportInterface, + &requestHeaders, + NULL, + 0, + &response, + 0 ); + TEST_ASSERT_EQUAL( HTTPSuccess, returnStatus ); +} + +/*-----------------------------------------------------------*/ + /* Test the buffer limit is reached on the network read, but the parser indicated * the response is not complete. */ void test_HTTPClient_Send_response_larger_than_buffer( void ) @@ -1132,7 +1177,8 @@ void test_HTTPClient_Send_network_error_request_headers( void ) { HTTPStatus_t returnStatus = HTTPSuccess; - sendErrorCall = 0U; + /* An error is returned from the transport send on the first call. */ + sendErrorCall = 1U; transportInterface.send = transportSendNetworkError; returnStatus = HTTPClient_Send( &transportInterface, &requestHeaders, @@ -1152,9 +1198,9 @@ void test_HTTPClient_Send_network_error_request_body( void ) transportInterface.send = transportSendNetworkError; - /* There is no Content-Length header written so the call to send an error on - * is call 1. */ - sendErrorCall = 1U; + /* The library sends the HTTP request body in the second call to + * the transport receive, if there are no errors or timeouts. */ + sendErrorCall = 2U; requestHeaders.pBuffer = ( uint8_t * ) ( HTTP_TEST_REQUEST_PUT_HEADERS ); requestHeaders.bufferLen = HTTP_TEST_REQUEST_PUT_HEADERS_LENGTH; requestHeaders.headersLen = HTTP_TEST_REQUEST_PUT_HEADERS_LENGTH; @@ -1178,7 +1224,8 @@ void test_HTTPClient_Send_less_bytes_request_headers( void ) http_parser_execute_Stub( http_parser_execute_whole_response ); transportInterface.send = transportSendLessThanBytesToWrite; - sendPartialCall = 0U; + /* Send the data partially in the first call to the transport send. */ + sendPartialCall = 1U; memcpy( requestHeaders.pBuffer, HTTP_TEST_REQUEST_PUT_HEADERS, HTTP_TEST_REQUEST_PUT_HEADERS_LENGTH ); @@ -1218,7 +1265,9 @@ void test_HTTPClient_Send_less_bytes_request_body( void ) transportInterface.send = transportSendLessThanBytesToWrite; - sendPartialCall = 1U; + /* The library will send the request body in the second call to transport + * write if there are no errors or timeouts. */ + sendPartialCall = 2U; memcpy( requestHeaders.pBuffer, HTTP_TEST_REQUEST_PUT_HEADERS, HTTP_TEST_REQUEST_PUT_HEADERS_LENGTH ); From 8803e82bb2089c1be3a9e5d9cd47c120f818d186 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 18:15:00 -0800 Subject: [PATCH 08/18] Add tests for coverage ofa sent timeout greater than zero. --- test/unit-test/core_http_send_utest.c | 110 ++++++++++++++++++-------- 1 file changed, 79 insertions(+), 31 deletions(-) diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index c1f4afa5..0542b388 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -199,6 +199,10 @@ static uint8_t sendErrorCall = 0; * to send less bytes than indicated. */ static uint8_t sendPartialCall = 0; +/* The tests set this variable to indicate at which call count of transport send + * to return zero from. */ +static uint8_t sendTimeoutCall = 0; + /* The network data to receive. */ static uint8_t * pNetworkData = NULL; /* The length of the network data to receive. */ @@ -211,7 +215,7 @@ static size_t firstPartBytes = 0; static uint8_t recvCurrentCall = 0; /* The test sets this variable to indicate which call count count of transport - * receive to return an error from. */ + * receive to return zero from. */ static uint8_t recvTimeoutCall = 0; /* The count of times a mocked http_parser_execute callback has been invoked. */ static uint8_t httpParserExecuteCallCount; @@ -232,7 +236,7 @@ static HTTPClient_ResponseHeaderParsingCallback_t headerParsingCallback = { 0 }; /* A mocked timer query function that increments on every call. */ static uint32_t getTestTime( void ) { - static entryTime = 0; + static uint32_t entryTime = 0; return entryTime++; } @@ -273,10 +277,22 @@ static int32_t transportSendSuccess( NetworkContext_t * pNetworkContext, const void * pBuffer, size_t bytesToWrite ) { + int32_t retVal = bytesToWrite; + ( void ) pNetworkContext; sendCurrentCall++; + if( sendTimeoutCall == sendCurrentCall ) + { + return 0; + } + + if( sendPartialCall == sendCurrentCall ) + { + retVal -= 1; + } + if( checkContentLength == 1U ) { if( sendCurrentCall == 1U ) @@ -290,7 +306,7 @@ static int32_t transportSendSuccess( NetworkContext_t * pNetworkContext, } } - return bytesToWrite; + return retVal; } /* Application transport send interface that returns a network error depending @@ -300,37 +316,16 @@ static int32_t transportSendNetworkError( NetworkContext_t * pNetworkContext, const void * pBuffer, size_t bytesToWrite ) { - ( void ) pNetworkContext; - ( void ) pBuffer; int32_t retVal = bytesToWrite; - sendCurrentCall++; - - if( sendErrorCall == sendCurrentCall ) - { - retVal = -1; - } - - return retVal; -} - -/* Application transport send interface that returns less bytes than expected - * depending on the call count. Set sendPartialCall to 0 to return less bytes on - * the first call. Set sendPartialCall to 1 to return less bytes on the second - * call. */ -static int32_t transportSendLessThanBytesToWrite( NetworkContext_t * pNetworkContext, - const void * pBuffer, - size_t bytesToWrite ) -{ ( void ) pNetworkContext; ( void ) pBuffer; - int32_t retVal = bytesToWrite; sendCurrentCall++; - if( sendPartialCall == sendCurrentCall ) + if( sendErrorCall == sendCurrentCall ) { - retVal -= 1; + retVal = -1; } return retVal; @@ -746,6 +741,7 @@ void setUp( void ) sendCurrentCall = 0; sendErrorCall = 0; sendPartialCall = 0; + sendTimeoutCall = 0; checkContentLength = 0; pNetworkData = ( uint8_t * ) HTTP_TEST_RESPONSE_HEAD; networkDataLen = HTTP_TEST_RESPONSE_HEAD_LENGTH; @@ -1091,9 +1087,9 @@ void test_HTTPClient_Send_timeout_partial_response( void ) /*-----------------------------------------------------------*/ -/* Test a timeout is received from the first network read, but the read after - * the response is received fully. */ -void test_HTTPClient_Send_timeout_recv_immediately_but_success_after( void ) +/* Test zero data is received, but we are able to receive again before the + * receive retry timeout. */ +void test_HTTPClient_Send_timeout_recv_retry( void ) { HTTPStatus_t returnStatus = HTTPSuccess; @@ -1216,6 +1212,58 @@ void test_HTTPClient_Send_network_error_request_body( void ) /*-----------------------------------------------------------*/ +/* Test zero data is sent, but we are able to send again before the + * send retry timeout. */ +void test_HTTPClient_Send_timeout_send_retry( void ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + http_parser_execute_Stub( http_parser_execute_whole_response ); + response.getTime = getTestTime; + + /* An zero is returned from the transport send on the first call. */ + sendTimeoutCall = 1U; + transportInterface.send = transportSendSuccess; + returnStatus = HTTPClient_Send( &transportInterface, + &requestHeaders, + NULL, + 0U, + &response, + 0U ); + TEST_ASSERT_EQUAL( HTTPSuccess, returnStatus ); +} + +/*-----------------------------------------------------------*/ + +/* Test zero data is partially sent, but we receive zero data in the next + * call. */ +void test_HTTPClient_Send_timeout_send_retry_fail( void ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + http_parser_execute_Stub( http_parser_execute_whole_response ); + + /* By default a HEAD request is ready to be sent. */ + transportInterface.send = transportSendSuccess; + /* Send the data partially in the first call to the transport send. */ + sendPartialCall = 1U; + + /* Timeout in the second call. Since there is no HTTPResponse_t.getTime + * function configured, we should never retry. */ + sendTimeoutCall = 2U; + + returnStatus = HTTPClient_Send( &transportInterface, + &requestHeaders, + NULL, + 0, + &response, + 0 ); + + TEST_ASSERT_EQUAL( HTTPNetworkError, returnStatus ); +} + +/*-----------------------------------------------------------*/ + /* Test less bytes, of the request headers, are sent than expected. */ void test_HTTPClient_Send_less_bytes_request_headers( void ) { @@ -1223,7 +1271,7 @@ void test_HTTPClient_Send_less_bytes_request_headers( void ) http_parser_execute_Stub( http_parser_execute_whole_response ); - transportInterface.send = transportSendLessThanBytesToWrite; + transportInterface.send = transportSendSuccess; /* Send the data partially in the first call to the transport send. */ sendPartialCall = 1U; memcpy( requestHeaders.pBuffer, @@ -1263,7 +1311,7 @@ void test_HTTPClient_Send_less_bytes_request_body( void ) http_parser_execute_Stub( http_parser_execute_whole_response ); - transportInterface.send = transportSendLessThanBytesToWrite; + transportInterface.send = transportSendSuccess; /* The library will send the request body in the second call to transport * write if there are no errors or timeouts. */ From 4991733c346cbfaf494452e42a9660b3619f3c72 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 18:19:25 -0800 Subject: [PATCH 09/18] Update the default timeouts to match coreMQTT. --- source/include/core_http_config_defaults.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index ab3a8a19..da3dbfc8 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -78,10 +78,10 @@ * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
- * Default value: `0` + * Default value: `10` */ #ifndef HTTP_RECV_RETRY_TIMEOUT_MS - #define HTTP_RECV_RETRY_TIMEOUT_MS ( 0U ) + #define HTTP_RECV_RETRY_TIMEOUT_MS ( 10U ) #endif /** @@ -97,10 +97,10 @@ * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
- * Default value: `0` + * Default value: `10` */ #ifndef HTTP_SEND_RETRY_TIMEOUT_MS - #define HTTP_SEND_RETRY_TIMEOUT_MS ( 0U ) + #define HTTP_SEND_RETRY_TIMEOUT_MS ( 10U ) #endif /** From a87604cf94fbd8f749ce1b13aa12f67150d3da9c Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 18:37:59 -0800 Subject: [PATCH 10/18] Add configs to the doxygen. --- docs/doxygen/pages.dox | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/doxygen/pages.dox b/docs/doxygen/pages.dox index 2b6e6418..c74d5f27 100644 --- a/docs/doxygen/pages.dox +++ b/docs/doxygen/pages.dox @@ -277,6 +277,12 @@ defined. @section HTTP_USER_AGENT_VALUE @copydoc HTTP_USER_AGENT_VALUE +@section HTTP_SEND_RETRY_TIMEOUT_MS +@copydoc HTTP_SEND_RETRY_TIMEOUT_MS + +@section HTTP_RECV_RETRY_TIMEOUT_MS +@copydoc HTTP_RECV_RETRY_TIMEOUT_MS + @section http_logerror LogError @copydoc LogError From e25cc9b78fe84ef791a12ef151ec9c793a530319 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 18:48:49 -0800 Subject: [PATCH 11/18] Undo transport interface changes/ --- lexicon.txt | 3 -- source/interface/transport_interface.h | 63 +++++--------------------- 2 files changed, 11 insertions(+), 55 deletions(-) diff --git a/lexicon.txt b/lexicon.txt index e469b264..821ed81c 100644 --- a/lexicon.txt +++ b/lexicon.txt @@ -9,7 +9,6 @@ api apis ascii aws -bool br bufferlen bufferlength @@ -18,7 +17,6 @@ bytesremaining bytessent bytestorecv bytestosend -calltlsrecvfunc cb cbmc chk @@ -231,7 +229,6 @@ tcpsocketcontext tls tlscontext tlsrecv -tlsrecvcount tlssend toascii totalreceived diff --git a/source/interface/transport_interface.h b/source/interface/transport_interface.h index ecfe2af0..e166bf1a 100644 --- a/source/interface/transport_interface.h +++ b/source/interface/transport_interface.h @@ -96,37 +96,16 @@ * size_t bytesToRecv ) * { * int32_t bytesReceived = 0; - * bool callTlsRecvFunc = true; - * - * // For a single byte read request, check if data is available on the network. - * if( bytesToRecv == 1 ) + * bytesReceived = TLSRecv( pNetworkContext->tlsContext, + * pBuffer, + * bytesToRecv, + * MY_SOCKET_TIMEOUT ); + * if( bytesReceived < 0 ) * { - * // If no data is available on the network, do not call TLSRecv - * // to avoid blocking for socket timeout. - * if( TLSRecvCount( pNetworkContext->tlsContext ) == 0 ) - * { - * callTlsRecvFunc = false; - * } + * // Handle socket error. * } + * // Handle other cases. * - * if( callTlsRecvFunc == true ) - * { - * bytesReceived = TLSRecv( pNetworkContext->tlsContext, - * pBuffer, - * bytesToRecv, - * MY_SOCKET_TIMEOUT ); - * if( bytesReceived < 0 ) - * { - * // If the error code represents a timeout, then the return - * // code should be translated to zero so that the caller - * // can retry the read operation. - * if( bytesReceived == MY_SOCKET_ERROR_TIMEOUT ) - * { - * bytesReceived = 0; - * } - * } - * // Handle other cases. - * } * return bytesReceived; * } * @endcode @@ -153,14 +132,7 @@ * pBuffer, * bytesToSend, * MY_SOCKET_TIMEOUT ); - * - * // If underlying TCP buffer is full, set the return value to zero - * // so that caller can retry the send operation. - * if( bytesSent == MY_SOCKET_ERROR_BUFFER_FULL ) - * { - * bytesSent = 0; - * } - * else if( bytesSent < 0 ) + * if( bytesSent < 0 ) * { * // Handle socket error. * } @@ -187,15 +159,6 @@ typedef struct NetworkContext NetworkContext_t; * @transportcallback * @brief Transport interface for receiving data on the network. * - * @note It is RECOMMENDED that the transport receive implementation - * does NOT block when requested to read a single byte. A single byte - * read request can be made by the caller to check whether there is a - * new frame available on the network for reading. - * However, the receive implementation MAY block for a timeout period when - * it is requested to read more than 1 byte. This is because once the caller - * is aware that a new frame is available to read on the network, then - * the likelihood of reading more than one byte over the network becomes high. - * * @param[in] pNetworkContext Implementation-defined network context. * @param[in] pBuffer Buffer to receive the data into. * @param[in] bytesToRecv Number of bytes requested from the network. @@ -204,10 +167,8 @@ typedef struct NetworkContext NetworkContext_t; * error. * * @note If no data is available on the network to read and no error - * has occurred, zero MUST be the return value. A zero return value - * SHOULD represent that the read operation can be retried by calling - * the API function. Zero MUST NOT be returned if a network disconnection - * has occurred. + * has occurred, zero MUST be the return value. Zero MUST NOT be used + * if a network disconnection has occurred. */ /* @[define_transportrecv] */ typedef int32_t ( * TransportRecv_t )( NetworkContext_t * pNetworkContext, @@ -227,9 +188,7 @@ typedef int32_t ( * TransportRecv_t )( NetworkContext_t * pNetworkContext, * * @note If no data is transmitted over the network due to a full TX buffer and * no network error has occurred, this MUST return zero as the return value. - * A zero return value SHOULD represent that the send operation can be retried - * by calling the API function. Zero MUST NOT be returned if a network disconnection - * has occurred. + * Zero MUST NOT be returned if a network disconnection has occurred. */ /* @[define_transportsend] */ typedef int32_t ( * TransportSend_t )( NetworkContext_t * pNetworkContext, From 241ab541d977bedc562ece14643bcb28e699f4e0 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 21:05:31 -0800 Subject: [PATCH 12/18] Fix some spellings. --- lexicon.txt | 1 + test/unit-test/core_http_send_utest.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lexicon.txt b/lexicon.txt index e469b264..a8547ce7 100644 --- a/lexicon.txt +++ b/lexicon.txt @@ -201,6 +201,7 @@ receivehttpdata recv recvcurrentcall recvstopcall +recvtimeoutcall reponse reqbodybuflen reqbodylen diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index 0542b388..e39c9800 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -1099,7 +1099,7 @@ void test_HTTPClient_Send_timeout_recv_retry( void ) /* Set the optional time keeping function to retry the receive when zero * data is read from the network. */ response.getTime = getTestTime; - /* On the first call to the transport recieve, return a zero. */ + /* On the first call to the transport receive, return a zero. */ recvTimeoutCall = 1; /* With HTTP_RECV_RETRY_TIMEOUT_MS set to greater than 1U in core_http_config.h From a4025a0f8855b738f20280749bc3db34ffd0d14f Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 21:20:21 -0800 Subject: [PATCH 13/18] Add some more docs about porting the new configs and hwo the config is disabled whtn the time keeping function does not exist. --- docs/doxygen/pages.dox | 17 +++++++++++++++++ source/include/core_http_config_defaults.h | 8 ++++++++ 2 files changed, 25 insertions(+) diff --git a/docs/doxygen/pages.dox b/docs/doxygen/pages.dox index 03b4ca21..7594797c 100644 --- a/docs/doxygen/pages.dox +++ b/docs/doxygen/pages.dox @@ -191,6 +191,8 @@ then the @ref HTTP_DO_NOT_USE_CUSTOM_CONFIG macro must be defined. The following macros can be configured for this library: - @ref HTTP_MAX_RESPONSE_HEADERS_SIZE_BYTES - @ref HTTP_USER_AGENT_VALUE + - @ref HTTP_SEND_RETRY_TIMEOUT_MS + - @ref HTTP_RECV_RETRY_TIMEOUT_MS In addition, the following logging macros are used throughout this library: - @ref LogError @@ -232,6 +234,21 @@ struct NetworkContext { // Fields necessary for the transport implementations, e.g. a TCP socket descriptor. }; @endcode + +@section http_porting_time Time Function +@brief The HTTP library optionally relies on a function to generate millisecond +timestamps, for the purpose of calculating the elapsed time between non-zero +network sends and receives. + +@see @ref HTTPClient_GetCurrentTimeFunc_t + +Platforms can supply a function capable of generating 32-bit timestamps of +millisecond resolution. These timestamps need not correspond with any real world +clock; the only requirement is that the difference between two timestamps must +be an accurate representation of the duration between them, in milliseconds. + +This function is used in conjunction with macros @ref HTTP_SEND_RETRY_TIMEOUT_MS +and @ref HTTP_RECV_RETRY_TIMEOUT_MS. */ /** diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index da3dbfc8..b17ff1d7 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -75,6 +75,10 @@ * * If the timeout expires, the #HTTPClient_Send function will return * #HTTPNetworkError. + * + * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS + * is unused. When this timeout is unused, #HTTPClient_Send will not retry the + * transport receive when it returns zero. * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
@@ -94,6 +98,10 @@ * transmission over the network through the transport send function. * * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. + * + * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS + * is unused. When this timeout is unused, #HTTPClient_Send will not retry the + * transport send when it returns zero. * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
From e654445e19f1066d14b07c64ffdc6053437858ab Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 22:50:15 -0800 Subject: [PATCH 14/18] Update CBMC proofs for the change. --- test/cbmc/include/core_http_config.h | 28 +++++++++++- test/cbmc/include/get_time_stub.h | 38 ++++++++++++++++ .../HTTPClient_Send/HTTPClient_Send_harness.c | 6 +++ test/cbmc/proofs/HTTPClient_Send/Makefile | 1 + test/cbmc/stubs/get_time_stub.c | 44 +++++++++++++++++++ 5 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 test/cbmc/include/get_time_stub.h create mode 100644 test/cbmc/stubs/get_time_stub.c diff --git a/test/cbmc/include/core_http_config.h b/test/cbmc/include/core_http_config.h index 4cdac4e6..fcbcd52d 100644 --- a/test/cbmc/include/core_http_config.h +++ b/test/cbmc/include/core_http_config.h @@ -28,6 +28,32 @@ #ifndef CORE_HTTP_CONFIG_H #define CORE_HTTP_CONFIG_H -/* Empty configuration header simply for compilation. */ +/** + * @brief The maximum duration between non-empty network reads while receiving + * an HTTP response via the #HTTPClient_Send API function. + * + * The transport receive function may be called multiple times until the end of + * the response is detected by the parser. This timeout represents the maximum + * duration that is allowed without any data reception from the network for the + * incoming response. + * + * If the timeout expires, the #HTTPClient_Send function will return + * #HTTPNetworkError. + */ +#define HTTP_RECV_RETRY_TIMEOUT_MS ( 1U ) + +/** + * @brief The maximum duration between non-empty network transmissions while + * sending an HTTP request via the #HTTPClient_Send API function. + * + * When sending an HTTP request, the transport send function may be called multiple + * times until all of the required number of bytes are sent. + * This timeout represents the maximum duration that is allowed for no data + * transmission over the network through the transport send function. + * + * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. + */ +#define HTTP_SEND_RETRY_TIMEOUT_MS ( 1U ) + #endif /* ifndef CORE_HTTP_CONFIG_H */ diff --git a/test/cbmc/include/get_time_stub.h b/test/cbmc/include/get_time_stub.h new file mode 100644 index 00000000..4e78a76e --- /dev/null +++ b/test/cbmc/include/get_time_stub.h @@ -0,0 +1,38 @@ +/* + * coreHTTP v1.0.0 + * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/** + * @file get_time_stub.h + * @brief Stub definition for the application defined callback to retrieve the + * current time in milliseconds. + */ +#ifndef GET_TIME_STUB_H_ +#define GET_TIME_STUB_H_ + +/** + * Application defined callback to retrieve the current time in milliseconds. + * + * @return The current time in milliseconds. + */ +uint32_t GetCurrentTimeStub( void ); + +#endif /* ifndef GET_TIME_STUB_H_ */ diff --git a/test/cbmc/proofs/HTTPClient_Send/HTTPClient_Send_harness.c b/test/cbmc/proofs/HTTPClient_Send/HTTPClient_Send_harness.c index c1f44c26..24d443c3 100644 --- a/test/cbmc/proofs/HTTPClient_Send/HTTPClient_Send_harness.c +++ b/test/cbmc/proofs/HTTPClient_Send/HTTPClient_Send_harness.c @@ -28,6 +28,7 @@ #include "core_http_client.h" #include "http_cbmc_state.h" #include "transport_interface_stubs.h" +#include "get_time_stub.h" void HTTPClient_Send_harness() { @@ -61,6 +62,11 @@ void HTTPClient_Send_harness() pTransportInterface->recv = nondet_bool() ? NULL : TransportInterfaceReceiveStub; } + if( pResponse != NULL ) + { + pResponse->getTime = nondet_boot() ? NULL : GetCurrentTimeStub; + } + HTTPClient_Send( pTransportInterface, pRequestHeaders, pRequestBodyBuf, diff --git a/test/cbmc/proofs/HTTPClient_Send/Makefile b/test/cbmc/proofs/HTTPClient_Send/Makefile index d9a8371e..fdeb4daf 100644 --- a/test/cbmc/proofs/HTTPClient_Send/Makefile +++ b/test/cbmc/proofs/HTTPClient_Send/Makefile @@ -69,6 +69,7 @@ PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE).c PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/HTTPClient_Send_http_parser_execute.c PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/transport_interface_stubs.c +PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/get_time_stub.c PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/strncpy.c PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/httpHeaderStrncpy.c diff --git a/test/cbmc/stubs/get_time_stub.c b/test/cbmc/stubs/get_time_stub.c new file mode 100644 index 00000000..eb53a2d1 --- /dev/null +++ b/test/cbmc/stubs/get_time_stub.c @@ -0,0 +1,44 @@ +/* + * coreHTTP v1.0.0 + * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/** + * @file get_time_stub.c + * @brief A stub to mock the retrieval of current time. + */ +#include "stdint.h" +#include "get_time_stub.h" + + +uint32_t GetCurrentTimeStub( void ) +{ + /* The HTTP relies on this timestamp in order to complete the network send + * and receive loops. Returning an unbounded timestamp does not add value to + * the proofs as the HTTP library uses this time stamp only in an arithmetic + * operation to get the difference. In C arithmetic operations on unsigned + * integers are guaranteed to reliably wrap around with no adverse side + * effects. If the time returned was unbounded, the network send and receive + * loops could be unwound a large number of times making the proof execution + * very long. */ + static uint32_t globalEntryTime = 0; + + return globalEntryTime++; +} From e2e23482f1e16c10c134e032867ed62ad2390c36 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Thu, 10 Dec 2020 22:51:17 -0800 Subject: [PATCH 15/18] Uncrustify. --- source/include/core_http_config_defaults.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index b17ff1d7..4cab6092 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -75,7 +75,7 @@ * * If the timeout expires, the #HTTPClient_Send function will return * #HTTPNetworkError. - * + * * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS * is unused. When this timeout is unused, #HTTPClient_Send will not retry the * transport receive when it returns zero. @@ -98,7 +98,7 @@ * transmission over the network through the transport send function. * * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. - * + * * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS * is unused. When this timeout is unused, #HTTPClient_Send will not retry the * transport send when it returns zero. From bb34fe25ef8436e5b35b5693c3ef9a1a994096b0 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Fri, 11 Dec 2020 11:23:27 -0800 Subject: [PATCH 16/18] Address PR comments. --- docs/doxygen/pages.dox | 13 +++++---- source/core_http_client.c | 34 ++++++++++++---------- source/include/core_http_client.h | 11 +++---- source/include/core_http_config_defaults.h | 4 +-- test/cbmc/include/core_http_config.h | 8 +++++ test/cbmc/stubs/get_time_stub.c | 4 +-- 6 files changed, 43 insertions(+), 31 deletions(-) diff --git a/docs/doxygen/pages.dox b/docs/doxygen/pages.dox index 7594797c..756e0907 100644 --- a/docs/doxygen/pages.dox +++ b/docs/doxygen/pages.dox @@ -237,15 +237,16 @@ struct NetworkContext { @section http_porting_time Time Function @brief The HTTP library optionally relies on a function to generate millisecond -timestamps, for the purpose of calculating the elapsed time between non-zero -network sends and receives. +timestamps, for the purpose of calculating the elapsed time when no data has +been sent or received. @see @ref HTTPClient_GetCurrentTimeFunc_t -Platforms can supply a function capable of generating 32-bit timestamps of -millisecond resolution. These timestamps need not correspond with any real world -clock; the only requirement is that the difference between two timestamps must -be an accurate representation of the duration between them, in milliseconds. +Applications can supply their platform-specific function capable of generating +32-bit timestamps of millisecond resolution. These timestamps need not correspond +with any real world clock; the only requirement is that the difference between two +timestamps must be an accurate representation of the duration between them, in +milliseconds. This function is used in conjunction with macros @ref HTTP_SEND_RETRY_TIMEOUT_MS and @ref HTTP_RECV_RETRY_TIMEOUT_MS. diff --git a/source/core_http_client.c b/source/core_http_client.c index 2932a09b..9c880407 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -193,7 +193,7 @@ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, * @param[in] pRequestHeaders Request headers for the corresponding HTTP request. * * @return Returns #HTTPSuccess if successful. #HTTPNetworkError for a transport - * receive error. Please see #parseHttpResponse, and #getFinalResponseStatus for + * receive error. Please see #parseHttpResponse and #getFinalResponseStatus for * other statuses returned. */ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pTransport, @@ -954,6 +954,7 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa const HTTPRequestHeaders_t * pRequestHeaders ) { assert( pParsingContext != NULL ); + assert( pRequestHeaders != NULL ); assert( pRequestHeaders->headersLen >= HTTP_MINIMUM_REQUEST_LINE_LENGTH ); /* Initialize the third-party HTTP parser to parse responses. */ @@ -968,10 +969,10 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa /* No response to update is associated with this parsing context yet. */ pParsingContext->pResponse = NULL; - /* The parsing context needs to know if the response is for a HEAD request. - * The third-party parser requires parsing is manually indicated to stop - * in the httpParserOnHeadersCompleteCallback() for a HEAD response, - * otherwise the parser will not indicate the message was complete. */ + /* The parsing context needs to know if the expected response is to a HEAD + * request. For a HEAD response, the third-party parser requires parsing is + * indicated to stop by returning a 1 from httpParserOnHeadersCompleteCallback(). + * If this is not done, the parser will not indicate the message is complete. */ if( strncmp( ( const char * ) ( pRequestHeaders->pBuffer ), HTTP_METHOD_HEAD, sizeof( HTTP_METHOD_HEAD ) - 1U ) == 0 ) @@ -1762,7 +1763,8 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, retryTimeoutMs = 0U; } - /* Start the last send time to allow retries on the first zero data sent. */ + /* Initialize the last send time to allow retries, if 0 bytes are sent on + * the first try. */ lastSendTimeMs = getTimestampMs(); /* Loop until all data is sent. */ @@ -1772,7 +1774,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, pIndex, bytesRemaining ); - /* A transport status of less than zero is an error. */ + /* BytesSent less than zero is an error. */ if( bytesSent < 0 ) { LogError( ( "Failed to send data: Transport send error: " @@ -1962,7 +1964,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT { HTTPStatus_t returnStatus = HTTPSuccess; size_t totalReceived = 0U; - int32_t currentReceived = 0U; + int32_t currentReceived = 0; HTTPParsingContext_t parsingContext = { 0 }; uint8_t shouldRecv = 1U, shouldParse = 1U, timeoutReached = 0U; uint32_t lastRecvTimeMs = 0U, timeSinceLastRecvMs = 0U; @@ -1984,8 +1986,8 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT retryTimeoutMs = 0U; } - /* Start the last receive time to allow retries on the first zero data - * receive. */ + /* Initialize the last send time to allow retries, if 0 bytes are sent on + * the first try. */ lastRecvTimeMs = pResponse->getTime(); while( shouldRecv == 1U ) @@ -2070,12 +2072,12 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT /*-----------------------------------------------------------*/ -HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, - HTTPClient_GetCurrentTimeFunc_t getTimestampMs, - HTTPRequestHeaders_t * pRequestHeaders, - const uint8_t * pRequestBodyBuf, - size_t reqBodyBufLen, - uint32_t sendFlags ) +static HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + const uint8_t * pRequestBodyBuf, + size_t reqBodyBufLen, + uint32_t sendFlags ) { HTTPStatus_t returnStatus = HTTPSuccess; diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index 8f074058..199db34e 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -454,12 +454,13 @@ typedef struct HTTPResponse /** * @brief Optional callback for getting the system time. * - * This is used to calculate the elapsed time between network sends and - * receives greater than zero. If this field is set to NULL, then network - * send and receive won't be retries after a zero is returned. + * This is used to calculate the elapsed time when retrying network reads or + * sends that return zero bytes received or sent, respectively. If this + * field is set to NULL, then network send and receive won't be retried + * after a zero is returned. * - * If this function is set, then the maximum elapsed time between network - * receives greater than zero is set in HTTP_RECV_RETRY_TIMEOUT_MS. + * If this function is set, then the maximum time for retrying network reads + * that return zero bytes can be set through #HTTP_RECV_RETRY_TIMEOUT_MS. * * If this function is set, then the maximum elapsed time between network * sends greater than zero is set in HTTP_SEND_RETRY_TIMEOUT_MS. diff --git a/source/include/core_http_config_defaults.h b/source/include/core_http_config_defaults.h index 4cab6092..08f61854 100644 --- a/source/include/core_http_config_defaults.h +++ b/source/include/core_http_config_defaults.h @@ -78,7 +78,7 @@ * * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS * is unused. When this timeout is unused, #HTTPClient_Send will not retry the - * transport receive when it returns zero. + * transport receive calls that return zero bytes read. * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
@@ -101,7 +101,7 @@ * * If #HTTPResponse_t.getTime is set to NULL, then this HTTP_RECV_RETRY_TIMEOUT_MS * is unused. When this timeout is unused, #HTTPClient_Send will not retry the - * transport send when it returns zero. + * transport send calls that return zero bytes sent. * * Possible values: Any positive 32 bit integer. A small timeout value * is recommended.
diff --git a/test/cbmc/include/core_http_config.h b/test/cbmc/include/core_http_config.h index fcbcd52d..60f8b902 100644 --- a/test/cbmc/include/core_http_config.h +++ b/test/cbmc/include/core_http_config.h @@ -39,6 +39,10 @@ * * If the timeout expires, the #HTTPClient_Send function will return * #HTTPNetworkError. + * + * This is set to 1 to exit right away after a zero is received in the transport + * receive stub. There is no added value, in proving memory safety, to repeat + * the logic that checks if the retry timeout is reached. */ #define HTTP_RECV_RETRY_TIMEOUT_MS ( 1U ) @@ -52,6 +56,10 @@ * transmission over the network through the transport send function. * * If the timeout expires, the #HTTPClient_Send function returns #HTTPNetworkError. + * + * This is set to 1 to exit right away after a zero is received in the transport + * send stub. There is no added value, in proving memory safety, to repeat + * the logic that checks if the retry timeout is reached. */ #define HTTP_SEND_RETRY_TIMEOUT_MS ( 1U ) diff --git a/test/cbmc/stubs/get_time_stub.c b/test/cbmc/stubs/get_time_stub.c index eb53a2d1..06025556 100644 --- a/test/cbmc/stubs/get_time_stub.c +++ b/test/cbmc/stubs/get_time_stub.c @@ -32,8 +32,8 @@ uint32_t GetCurrentTimeStub( void ) { /* The HTTP relies on this timestamp in order to complete the network send * and receive loops. Returning an unbounded timestamp does not add value to - * the proofs as the HTTP library uses this time stamp only in an arithmetic - * operation to get the difference. In C arithmetic operations on unsigned + * the proofs as the HTTP library uses this timestamp only in an arithmetic + * operation to get the difference. In C, arithmetic operations on unsigned * integers are guaranteed to reliably wrap around with no adverse side * effects. If the time returned was unbounded, the network send and receive * loops could be unwound a large number of times making the proof execution From a2908dfdd624c38fd59a8c32f8d8f8cad15bc573 Mon Sep 17 00:00:00 2001 From: Sarena Meas Date: Fri, 11 Dec 2020 11:33:59 -0800 Subject: [PATCH 17/18] Fix build errors left in logging. --- source/core_http_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 9c880407..4b051e3c 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -1786,7 +1786,7 @@ static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, /* It is a bug in the application's transport send implementation if * more bytes than expected are sent. To avoid a possible overflow * in converting bytesRemaining from unsigned to signed, this assert - * must exist after the check for transportStatus being negative. */ + * must exist after the check for bytesSent being negative. */ assert( ( size_t ) bytesSent <= bytesRemaining ); /* Record the most recent time of successful transmission. */ @@ -2002,7 +2002,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT { LogError( ( "Failed to receive HTTP data: Transport recv() " "returned error: TransportStatus=%ld", - ( long int ) transportStatus ) ); + ( long int ) currentReceived ) ); returnStatus = HTTPNetworkError; /* Do not invoke the parser on network errors. */ From 871ab8f315a1399f4a187eb82fdf98f94b2e8849 Mon Sep 17 00:00:00 2001 From: SarenaAWS <6563840+sarenameas@users.noreply.github.com> Date: Fri, 11 Dec 2020 12:08:20 -0800 Subject: [PATCH 18/18] Update test/unit-test/core_http_send_utest.c --- test/unit-test/core_http_send_utest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index e39c9800..c6afdb0c 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -1235,7 +1235,7 @@ void test_HTTPClient_Send_timeout_send_retry( void ) /*-----------------------------------------------------------*/ -/* Test zero data is partially sent, but we receive zero data in the next +/* Test data is partially sent, but we receive zero data in the next * call. */ void test_HTTPClient_Send_timeout_send_retry_fail( void ) {