Unable to handle responses without a Content-Length header or chunked transfer encoding

[AguileraG]

I am using coreHTTP v3.0.0 to communicate with an HTTP/1.1 server that does not include a Content-Length header nor uses chunked transfer encoding. In this case, the server tells the client that the response body has been fully read by closing the underlying TCP connection. Using BSD sockets, this would mean that select() informs that data can be read from the socket, but the subsequent call to recv() returns no data.

Unfortunately, coreHTTP is not able to handle this behaviour as it is indistinguishable from a read timeout. I can think of two fixes that could be applied to solve this issue, but both would require modifying TransportRecv_t in some way:

1. Add a new return value to TransportRecv_t that would be set if the transport connection has been closed. This solution is clearer, but would change the signature of TransportRecv_t.
2. Use a specific TransportRecv_t return value to represent a closed connection. This keeps the API but may cause issues if the transport implementation uses this return value.

In addition, either parseHttpResponse() or httpParserOnBodyCallback() should be modified to call to llhttp_finish() if required. This change may look something like this:

    /* 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 ) );
    }

Would it be possible to fix this issue without the TransportRecv_t modifications? I have implemented one of the possible fixes in my fork of coreHTTP, in case you want to have a look.

[xuelix]

Thanks for raising the issue, we'll have a team member to review your suggestion.

[ericbj29]

Hi. As you pointed out, the current transport interface cannot distinguish between the end of a file and no data. This is because the library does not define any errors. Our preferred solution would be to define error codes (one of which is HTTP_RECV_NO_DATA) and return these instead of 0. The error codes could be defined in an enum with three values: HTTP_RECV_SUCCESS, HTTP_RECV_ERROR, and HTTP_RECV_NO_DATA. Under this design, bytesToRecv (or, as suggested below bytesReceived) would only be valid in the case of HTTP_RECV_SUCCESS. The behavior in each of the cases would be as follows:

• On HTTP_RECV_SUCCESS: If non-zero length is received, then current behavior. Otherwise, if 0 bytes are read, then finish reading.
• On HTTP_RECV_ERROR: Current negative value behavior.
• On HTTP_RECV_NO_DATA: Current 0 behavior.

This is a breaking change, so we would like to take advantage of the moment and make an additional update. In particular, we would like to separate the error and length like so:

typedef COREHTTP_RECV_ERROR ( * TransportRecv_t )( NetworkContext_t * pNetworkContext,
                                                   void * pBuffer,
                                                   size_t bytesToRecv,
                                                   size_t * bytesReceived );

We would greatly appreciate a pull request implementing these suggestions. Otherwise, they are on our roadmap.

[cryi]

This should be solvable with #172 actually. It will definitely need some code on the host side but should be doable. There is essential info in the PR comments.