freertos / corehttp Goto Github PK

Hi,
I am working on small websocket library that is using transport_interface layer and coreHTTP library for initial handshaking.

"Upgrade:" header in HTTP response ends with error code from function HTTPClient_Send.

I made my fix in file core_http_client.c on line 1206:

if(llhttp_execute( &( pParsingContext->llhttpParser ), parsingStartLoc, parseLen ) == HPE_PAUSED_UPGRADE)
llhttp_resume_after_upgrade(&( pParsingContext->llhttpParser ));

Regards,
Peter.

Description

We are looking to build more sophisticated applications on top of the coreHTTP library. Our use case requires support for chunked body reads and writes, streaming, and other advanced HTTP functionalities. However, the current encapsulation of sendHttpHeaders, sendHttpData, receiveAndParseHttpResponse functions restricts our ability to extend or adapt the library to our needs.

Solution

We propose exposing the following internal functions as part of the public API:

• sendHttpHeaders
• sendHttpData
• receiveAndParseHttpResponse

By making these functions accessible, developers can have more control over the HTTP communication process, allowing for more complex use cases, such as handling chunked transfers, streaming data, and more.

Alternatives

Currently, we are patching coreHTTP during the build. This is a relatively easy patch so we can apply it during the build process and use it as if these functions are built into the core library. However, having these functions exposed officially in the library would provide a more robust and community-supported solution.

This is direct evolution of #160.

NOTE: We are right now testing this on portion of our production services as replacement for libcurl. We managed to drop binary size by ~30% while maintaining feature set.

Warnings related to strncpy are observed in the core_http_client.c file at the below lines of code. Attached the warnings snapshot :

Line 1:
( void ) strncpy( pBufferCur,
HTTP_HEADER_FIELD_SEPARATOR,
HTTP_HEADER_FIELD_SEPARATOR_LEN );

Line 2:
( void ) strncpy( pBufferCur,
HTTP_HEADER_END_INDICATOR,
HTTP_HEADER_END_INDICATOR_LEN );

Line 3:
( void ) strncpy( pBufferCur,
HTTP_HEADER_LINE_SEPARATOR,
HTTP_HEADER_LINE_SEPARATOR_LEN );

I'm trying to build coreHTTP in Ubuntu 23.10 but I'm getting this error:

raphy@raohy:~/libpeer/dependencies/coreHTTP$ gcc -I source/include/ -I source/interface/ -I source/dependency/3rdparty/llhttp/build/ -DHTTP_DO_NOT_USE_CUSTOM_CONFIG source/dependency/3rdparty/llhttp/build/libllhttp.a  source/dependency/3rdparty/llhttp/src/native/api.c source/dependency/3rdparty/llhttp/src/native/http.c -o coreHTT
/usr/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/13/../../../x86_64-linux-gnu/Scrt1.o: in function `_start':
(.text+0x1b): undefined reference to `main'
/usr/bin/ld: /tmp/cc5Ef6az.o: in function `llhttp_init':
api.c:(.text+0x1f): undefined reference to `llhttp__internal_init'
/usr/bin/ld: /tmp/cc5Ef6az.o: in function `llhttp_reset':
api.c:(.text+0x10f): undefined reference to `llhttp__internal_init'
/usr/bin/ld: /tmp/cc5Ef6az.o: in function `llhttp_execute':
api.c:(.text+0x178): undefined reference to `llhttp__internal_execute'
collect2: error: ld returned 1 exit status
raphy@raohy:~/libpeer/dependencies/coreHTTP$ 

What am I doing wrong? How to make it work?

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.

Dependency 3rd party module http-parser is unmaintained now, please see nodejs/http-parser#522
Please consider switching or remove dependency.

httpParserOnStatusCallback() is not called if a server returns a status code without a reason string such as following.

HTTP/1.1 400
Server: nginx
Date: Fri, 05 Jan 2024 05:25:21 GMT
Content-Type: application/json
Content-Length: 30
Connection: keep-alive
・・・

If a server returns "HTTP/1.1 400 Bad Request" instead of "HTTP/1.1 400", httpParserOnStatusCallback() is called.

Warning related to strncpy is observed in the core_http_client.c file at the below line of code. Attached is the warning snapshot :

Line of code:

( void ) strncpy( rangeValueBuffer,
HTTP_RANGE_REQUEST_HEADER_VALUE_PREFIX,
HTTP_RANGE_REQUEST_HEADER_VALUE_PREFIX_LEN );

Note: A similar issue was created in the past for which "strncpy" was replaced with "memcpy" as a fix. Attaching the link to that issue below.
#133

Description

CoreHTTP may rely upon state - state which may be accessible across FreeRTOS tasks. We want to verify coreHTTP is task/thread safe by ensuring that state access across FreeRTOS tasks is done so behind a synchronization mechanism.

Help is wanted to audit the repository for state which can be used outside of the CoreHTTP task/thread or functions.

This work is being tracked by the FreeRTOS team however it may not be prioritized for some time. Your help in identifying problem state variables/structures will help us in correcting these issues faster. As always, if you are able to fix the issue, we appreciate the help and will gladly look at your PR.

Related Issues

• OTA library state issue

cmake -S test -B build -DBUILD_CLONE_SUBMODULES=ON
CMake Error: The source directory "/home/jaiprakash/Desktop/coreHTTP/build" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.

Hello,

I've noticed that corehttp currently doesn't have a feature for reading the body in chunks. While examining the source, particularly the httpParserOnBodyCallback section, I'm curious if it would be possible to introduce a mechanism that allows the body to be read in distinct chunks.

The envisioned callback could return:

• A value less than 0 in case of an error.
• A value greater than 0 to indicate the number of bytes processed or written to the buffer.
• A return value of 0 to indicate that all is well, but no further action is needed.

By enabling chunked reading, users can easily manage large responses or work with limited buffer sizes more effectively.

I'd love to hear your thoughts on this suggestion.

Thank you!

P.S.: I am integrating corehttp as a basic replacement for libcurl. I'm uncertain about the extent of support for range headers. Thus, I'm exploring potential solutions.