Skip to content

Commit

Permalink
Extend TransportInterface_t to differenciate between read timeouts an…
Browse files Browse the repository at this point in the history
…d closed connections

Some HTTP servers do not include a Content-Length header nor use chunked
transfer encoding in their responses. In this situation, the server
closes the underlying TCP connection to tell the client that the
response has been completely read.

Currently, this behaviour is not supported by coreHTTP, as llhttp is not
able to detect the end of the response unless llhttp_finish() is called.

This issue can be solved by adding a new return value to TransportRecv_t
that allows coreHTTP to detect a closed TCP connection after all data
has been read to then call llhttp_finish().
  • Loading branch information
AguileraG committed Nov 27, 2023
1 parent 668db4b commit 8a65d39
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
25 changes: 20 additions & 5 deletions source/core_http_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ 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] isClosed Set to 1 if the peer has closed the connection.
*
* @return One of the following:
* - #HTTPSuccess
Expand All @@ -324,7 +325,8 @@ static void initializeParsingContextForFirstResponse( HTTPParsingContext_t * pPa
*/
static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext,
HTTPResponse_t * pResponse,
size_t parseLen );
size_t parseLen,
unsigned int isClosed );

/**
* @brief Callback invoked during llhttp_execute() to indicate the start of
Expand Down Expand Up @@ -1123,7 +1125,8 @@ static HTTPStatus_t processLlhttpError( const llhttp_t * pHttpParser )

static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext,
HTTPResponse_t * pResponse,
size_t parseLen )
size_t parseLen,
unsigned int isClosed )
{
HTTPStatus_t returnStatus;
const char * parsingStartLoc = NULL;
Expand Down Expand Up @@ -1180,6 +1183,15 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext,
llhttp_resume_after_upgrade( &( pParsingContext->llhttpParser ) );
}

/* Finish parsing if the connection has been closed by the server after
* the response has been sent. This is only relevant if the response
* does not include a Content-Length header nor uses chunked transfer
* encoding. */
if( llhttp_message_needs_eof( &( pParsingContext->llhttpParser ) ) && ( isClosed == 1U ) )
{
( void ) llhttp_finish( &( pParsingContext->llhttpParser ) );
}

/* The next location to parse will always be after what has already
* been parsed. */
pParsingContext->pBufferCur = parsingStartLoc + parseLen;
Expand Down Expand Up @@ -1993,6 +2005,7 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t
uint8_t shouldRecv = 1U, shouldParse = 1U, timeoutReached = 0U;
uint32_t lastRecvTimeMs = 0U, timeSinceLastRecvMs = 0U;
uint32_t retryTimeoutMs = HTTP_RECV_RETRY_TIMEOUT_MS;
unsigned int isClosed = 0U;

assert( pTransport != NULL );
assert( pTransport->recv != NULL );
Expand All @@ -2019,7 +2032,8 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t
/* Receive the HTTP response data into the pResponse->pBuffer. */
currentReceived = pTransport->recv( pTransport->pNetworkContext,
pResponse->pBuffer + totalReceived,
pResponse->bufferLen - totalReceived );
pResponse->bufferLen - totalReceived,
&isClosed );

/* Transport receive errors are negative. */
if( currentReceived < 0 )
Expand Down Expand Up @@ -2050,7 +2064,7 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t
{
timeSinceLastRecvMs = pResponse->getTime() - lastRecvTimeMs;
/* Do not invoke the response parsing for intermediate zero data. */
shouldParse = 0U;
shouldParse = ( isClosed == 1U ) ? 1U : 0U;

/* Check if the allowed elapsed time between non-zero data has been
* reached. */
Expand All @@ -2073,7 +2087,8 @@ HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t
* know that the value is greater than 0 we don't need to worry about int overflow. */
returnStatus = parseHttpResponse( &parsingContext,
pResponse,
( uint64_t ) currentReceived );
( uint64_t ) currentReceived,
isClosed );
}

/* Reading should continue if there are no errors in the transport receive
Expand Down
4 changes: 3 additions & 1 deletion source/interface/transport_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ typedef struct NetworkContext NetworkContext_t;
* @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.
* @param[out] isClosed Set to 1 if the peer has closed the connection.
*
* @return The number of bytes received or a negative value to indicate
* error.
Expand All @@ -220,7 +221,8 @@ typedef struct NetworkContext NetworkContext_t;
/* @[define_transportrecv] */
typedef int32_t ( * TransportRecv_t )( NetworkContext_t * pNetworkContext,
void * pBuffer,
size_t bytesToRecv );
size_t bytesToRecv,
unsigned int * isClosed );
/* @[define_transportrecv] */

/**
Expand Down

0 comments on commit 8a65d39

Please sign in to comment.