HTTP 1.1 server has two ways of indicating the message body length -
either by providing the Content Length header, or by closing the
connection when the entire body has been transmitted.
The second method didn't work with Zephyr's HTTP client implementation,
as EOF on a socket was treated as an error condition. Therefore, if no
Content Length was provided by the server, such transfers would always
end up with ECONNRESET error.
In order to fix this, we need to notify the parser about the EOF on a
socket when connection is closed. It is the parser role to determine
whether the EOF was expected in current state (by marking end of message
flag) or not (by setting an error).
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
POLLHUP event may be returned even if there is still data to read
and POLLIN is set. To ensure all data is consumed, check for
POLLHUP after handling POLLIN.
https://man7.org/linux/man-pages/man2/poll.2.html
Signed-off-by: Juha Ylinen <juha.ylinen@nordicsemi.no>
Update the response callback function signature to allow the callback to
return an error code, which in turn will cause the HTTP client to abort
the download.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Response "data_len" field needs to be set with the size of the
received data before calling the parser as it's used inside the on_body
callback, this commit fixes it.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The HTTP client would read data from a socket up to the size of the
receiving buffer, however it may not process them all. This is usually
only the case if protocol switching takes place, where the data read may
belong to another protocol. Therefore we need a way to notify the caller
about any potential data that is already present in the buffer and needs
to be processed.
Introduce an new data_len member in struct http_request to provide the
information about the data already available in the receive buffer. If,
after HTTP response processing, the value is non-zero, the data will be
available in the beginning of the user provided receive buffer.
To make this possible however, we need to track how many bytes were
actually processed by the http_parser, therefore the code will no longer
ignore the http_parser_execute() return value. To simplify processing,
it's also been changed how the receive buffer is used. Instead of using
it in a ring-buffer-like way, the offset variable will track how many
bytes are available in the buffer, and in the rare occasions when not
all data from the buffer was processed by the HTTP parser, we'll
memmomve the remaining data to the beginning of the buffer.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Fix handling of poll setting socket error flag.
In this case errno is no set, so should not be the return value either.
Instead retrieve the socket error for SOCKERR and return EBADF for
SOCKNVAL.
Signed-off-by: Joakim Andersson <joerchan@gmail.com>
When HTTP 101 Switching Protocols response is received, force the HTTP
parser to ignore any potential message body payload (which should
already belong to the new protocol).
This is usually not an issue, as Switching Protocols reply should
contain headers only, however it's been observed that some servers
specify chunked transfer-encoding header which tricks the parser
to wait for payload event though it never arrives.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Content-Range functionality added in recent commits has been propagated
to http_client module. If "Content-Range" string is detected on header
field, Content-Range are returned via http_response structure.
Signed-off-by: Piotr Radecki <piotr.radecki@jrdltd.co.uk>
GCC 12.2.0 can give this warning (version 11.4.0 did not), when
CONFIG_SPEED_OPTIMIZATIONS=y
subsys/net/lib/http/http_client.c: In function 'http_send_data.constprop':
subsys/net/lib/http/http_client.c:114:33: warning: 'strncpy' specified
bound depends on the length of the source argument [-Wstringop-truncation]
114 | strncpy(send_buf + end_of_send,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
115 | data + end_of_data,
| ~~~~~~~~~~~~~~~~~~~
116 | remaining);
| ~~~~~~~~~~
subsys/net/lib/http/http_client.c:87:41: note: length computed here
87 | remaining_len = strlen(data + end_of_data);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
The code properly checks that we do not overwrite the destination
buffer even if we use the source buffer length to determine how much
to copy. One possible fix is to use memcpy() or strcpy(), I opted
to use memcpy() because it has the length option which feels more
natural.
Fixes#79326
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
The http_client_req() function's timeout parameter is allowed
to be SYS_FOREVER_MS. However, K_MSEC() does not convert this
to a proper k_timeout_t, so sys_timepoint_calc() ends up
returning 0, which is causes immediate timeouts.
Check for this case specifically and force value passed to
sys_timepoint_calc() to be correct.
Signed-off-by: Pete Skeggs <peter.skeggs@nordicsemi.no>
If we couldn't send all (or any data) via the socket,
invoke poll instead of blindly retrying and flooding the socket.
Respect timeout through http_client_req
Signed-off-by: Andrey Dodonov <Andrey.Dodonov@endress.com>
Original code developed as a GSoC 2023 project by Emna Rekik.
Code refactored in order to provide better bisectability
as the origical commits were not bisectable.
The server supports static and dynamic resources, managed by
HTTP_SERVICE/HTTP_RESOURCE macros.
Fixes#59685Fixes#59686Fixes#59688Fixes#59690Fixes#59670Fixes#59700Fixes#59684Fixes#59693Fixes#59693Fixes#59694Fixes#59699Fixes#59696Fixes#59688Fixes#59690Fixes#59670Fixes#59700Fixes#59685Fixes#59686Fixes#59688Fixes#59691
Signed-off-by: Emna Rekik <emna.rekik007@gmail.com>
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Correct various small edge-case behaviors that have been accidentally
introduced in the http_client.
- http_client_req no longer incorrectly returns -ETIMEDOUT on NULL HTTP
resonse. -ETIMEDOUT is now only returned when the underlying TLS
socket times out.
- http_client_req now returns -ECONRESET upon incomplete (but non-NULL)
HTTP response. The request callback is no longer called in this case
(as with any other error state).
- http_wait_data has been refactored slightly to increase clarity.
Signed-off-by: Georges Oates_Larsen <georges.larsen@nordicsemi.no>
Return error to the caller if no data was received or there
was some other error. Earlier we did not check the error
condition properly.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Use internal constant ZSOCK_POLLIN instead of POLLIN to
make the http_client source file compile without error with
CONFIG_NET_SOCKETS_POSIX_NAMES disabled.
Fixes#55423
Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
Change the http timeout mechanism to use poll instead of shutdown.
This should fix a problem where the shutdown will be called in a
different thread context which can lead to deadlocks on certain
driver implementations like offloaded modem drivers.
Fixes#53967
Signed-off-by: Wouter Cappelle <wouter.cappelle@crodeon.com>
Some minor housekeeping prior to adding an http server
implementation. There are already a number of http headers
and that number will likely increase with subsequent work.
Moving them into a common directory cleans up the
`include/net` directory a bit.
Signed-off-by: Christopher Friedt <cfriedt@meta.com>
Logging v1 has been removed and log_strdup wrapper function is no
longer needed. Removing the function and its use in the tree.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
In order to bring consistency in-tree, migrate all subsystems code to
the new prefix <zephyr/...>. Note that the conversion has been scripted,
refer to zephyrproject-rtos#45388 for more details.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
http_client_req() was supposed to return the number of bytes sent as a
HTTP request. The return value was not riht however due to some bugs in
helper functions:
* http_send_data() returned the current buffer position istead of the
number of bytes actually sent. This could result in counting the same
data into the total request size several times. A helper variable was
added to track how many bytes were actually sent to the network.
* http_flush_data() forwarded the return value of sendall() helper
function. That function however did not return number of bytes sent,
but 0 or a negative error code.
Additionally, change the return type of sendall() function - according
to standard the ssize_t type is only capable of holding -1 negative
value, but the function could return the full range of negative errno
values. Use int instead.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Following #42026, the body_start pointer now points to the
start of the body fragment in the recv_buffer as long as there
is body in it, either entirely or partially.
Rename the body_start to body_frag_start to better reflect
what it represents.
Signed-off-by: Yong Cong Sin <yongcong.sin@gmail.com>
Adds an official behavior in response to null response from HTTP
endpoint.
Fixes#42988
Signed-off-by: Georges Oates_Larsen <georges.larsen@nordicsemi.no>
So far close() was called on underlying socket when timeout has expired.
This is wrong in several ways. First of all POSIX specification of
close() does not specify what should happen on blocking recv() call and
different systems have different behaviors (e.g. Linux does not wake up
recv() caller if there was no new incoming data, while other systems
might wakeup recv() caller immediately). Another (and much more severe)
problem is that HTTP client user does not know whether underlying socket
was already closed or not after HTTP request has finished. As a result
it was not clear whether close() should be called by HTTP client user.
Use shutdown(..., SHUT_RD) in internal HTTP client implementation, so
that recv() is woken up immediately with 0 as result (which means EOF).
This will allow to gracefully handle timeouts and make it clear that it
is application responsibility to always call close() after HTTP
request (successful or not).
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
A common pattern here was to take the work item as the subfield of a
containing object. But the contained field is not a k_work, it's a
k_work_delayable.
Things were working only because the work field was first, so the
pointers had the same value. Do things right and fix things to
produce correct code if/when that field ever moves within delayable.
Signed-off-by: Yong Cong Sin <yongcong.sin@gmail.com>
Set `body_start` pointer regardless of the body position in the recv
buffer. In result, the pointer shall indicate correctly position of the
body for each fragment, it's also explicit now that if the pointer is
not set for a fragment, there's no body in that particular fragment.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
HTTP_DATA_FINAL was incorrectly notified in case Content Length field
was present in the HTTP respone - in such case it was set for every
response fragment, not only the last one.
Fix this by relying on `message_complete` flag instead of
`http_should_keep_alive()` function to determine whether to notify
HTTP_DATA_FINAL or not. As the HTTP parser calls the
`on_message_complete()` callback in either case (response is chunked or
not), this seems to be a more reasonable apporach to determine whether
the fragment is final or not.
Additinally, instead of calling response callback for
`on_body`/`on_message_complete` separately, call it directly from
`http_wait_data()` function, after the parsing round. This fixes the
case when headers were not reported correctly when the provided buffer
was smaller than the total headers length, resulting in corrupted data
being reported to the user.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Using zsock_ in http_client instead of the POSIX API versions of the
functions allows the usage of http_client in combination with
CONFIG_POSIX_API.
Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Call on_status if the Reason-Phrase is not provided.
This allows for the numeric status code to be set.
Also, ensure the numeric status code is always set
in on_status, not just if the specific callback is set.
Signed-off-by: Justin Morton <justin.morton@nordicsemi.no>
Add numeric http status code to the response struct to allow for
easier processing by the caller. Textual status already exists.
Signed-off-by: Justin Morton <justin.morton@nordicsemi.no>
Replace all existing deprecated API with the recommended alternative.
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Bug fix and improved `payload` handling in `http_client_req`.
Changes to `http_client_req` behaviour:
If the user provides `payload_len` it is used to generate the
`Content-Length` header. This is done even if `payload_cb` is used to
provide the actual data. If no `payload_len` is specified then no
`Content-Length` is generated.
If `payload_cb` is provided it is called to send the payload data.
Otherwise `payload` is used as the payload buffer and sent. If
`payload_len` is not zero, it is used as the size of `payload`.
Otherwise `payload` is assumed to be a string and `strlen` is used to
determine its size. This is to maintain current behaviour and not break
existing samples.
Fixes#24431
Signed-off-by: Arvin Farahmand <arvinf@ip-logix.com>
Use k_timeout_t internally, no change to user API.
Clarify the documentation of the timeout parameter that it is
in milliseconds.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
The old legacy APIs use net-app library and as that is being
removed, then the dependencies need to be removed also.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Remove network specific default and max log level setting
and start to use the zephyr logging values for those.
Remove LOG_MODULE_REGISTER() from net_core.h and place the
calls into .c files. This is done in order to avoid weird
compiler errors in some cases and to make the code look similar
as other subsystems.
Fixes#11343Fixes#11659
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
As the debugging print calls are async, all the strings that might
be overwritten must use log_strdup() which will create a copy
of the printable string.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
The return of memset is never checked. This patch explicitly ignore
the return to avoid MISRA-C violations.
The only directory excluded directory was ext/* since it contains
only imported code.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
net_app_ctx maintains multiple net contexts(net_ctx). But when http
api's wants to reply or send some data, its always choose the first
net_context in the array, which is not correct always.
net_app_get_net_pkt_with_dst() api will select proper context
based on destination address. So with the help of new api in
net_app, http can select proper context and send packets. To
achieve this, desination address is provided in http_recv_cb_t
and http_connect_cb_t callbacks. Also chaged relevant API's to
provide destination address in http message preparation methods.
Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>
Logic for sending chunks of data is incompatible with adding
Content-Length: header.
Per https://tools.ietf.org/html/rfc7230#section-3.3.1:
"A sender MUST NOT send a Content-Length header field in any
message that contains a Transfer-Encoding header field."
Going a bit further in my mind: also don't send Transfer-Encoded
chunked data either when the Content-Length header is present.
In general, there will be problems if the http client library
makes payload changes without the user code knowing about it.
This patch removes the use of http_send_chunk() from the new
HTTP client code and instead sends the payload directly to
http_prepare_and_send()
This fixes an issue where every available buffer would be allocated
with repeating payload data because the for loop in http_request()
wasn't ending until we ran out of memory.
NOTE: This patch was previously applied but was lost when
commit d1675bf3e6 ("net: http: Remove the old legacy API")
moved code around.
Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
We should not use the user suppied timeout setting in
http_client_send_req() for the connection timeout. In the
previous API the call to tcp_connect() used
CONFIG_HTTP_CLIENT_NETWORK_TIMEOUT as the timeout setting.
Let's do that here too.
This fixes -ETIMEDOUT error generation when using K_NO_WAIT
for http_client_send_req().
NOTE: This patch was previously applied but was lost when
commit d1675bf3e6 ("net: http: Remove the old legacy API")
moved code around.
Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
In previous version of the HTTP API, commit 8ebaf29927 ("net: http:
dont timeout on HTTP requests w/o body") fixed handling of HTTP
responses without body content.
For the new API, let's add a specific fix for when PUT/POST requests
are responded to with just the status code.
Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Robert Lubos
Juha Ylinen
Joakim Andersson
Piotr Radecki
Jukka Rissanen
Pete Skeggs
Andrey Dodonov
Georges Oates_Larsen
Lucas Dietrich
Wouter Cappelle
Christopher Friedt
Krzysztof Chruscinski
Gerard Marull-Paretas
Sjors Hettinga
Yong Cong Sin
Marcin Niestroj
Benedikt Schmidt
Justin Morton
Peter Bigot
Flavio Ceolin
Jackie Ja
NavinSankar Velliangiri
Kumar Gala
Jukka Rissanen
Arvin Farahmand
Patrik Flykt
Ravi kumar Veeramally
Michael Scott