From 7600869c35342abb5219211f133b44910b2e577c Mon Sep 17 00:00:00 2001 From: Shubham Kulkarni Date: Fri, 18 Feb 2022 16:25:02 +0530 Subject: [PATCH] esp_http_client: Cache received data in http_on_body callback. This change fixes issue with data loss when multiple body chunks are received after calling esp_http_client_fetch_headers. --- components/esp_http_client/esp_http_client.c | 50 ++++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index cfdfb12771..511618d989 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -32,6 +32,7 @@ typedef struct { char *data; /*!< The HTTP data received from the server */ int len; /*!< The HTTP data len received from the server */ char *raw_data; /*!< The HTTP data after decoding */ + char *orig_raw_data;/*!< The Original pointer to HTTP data after decoding */ int raw_len; /*!< The HTTP data len after decoding */ char *output_ptr; /*!< The destination address of the data to be copied to after decoding */ } esp_http_buffer_t; @@ -74,6 +75,7 @@ typedef enum { HTTP_STATE_REQ_COMPLETE_HEADER, HTTP_STATE_REQ_COMPLETE_DATA, HTTP_STATE_RES_COMPLETE_HEADER, + HTTP_STATE_RES_ON_DATA_START, HTTP_STATE_RES_COMPLETE_DATA, HTTP_STATE_CLOSE } esp_http_state_t; @@ -115,6 +117,7 @@ struct esp_http_client { bool is_async; esp_transport_keep_alive_t keep_alive_cfg; struct ifreq *if_name; + unsigned cache_data_in_fetch_hdr: 1; }; typedef struct esp_http_client esp_http_client_t; @@ -265,10 +268,24 @@ static int http_on_body(http_parser *parser, const char *at, size_t length) { esp_http_client_t *client = parser->data; ESP_LOGD(TAG, "http_on_body %d", length); - client->response->buffer->raw_data = (char *)at; + if (client->response->buffer->output_ptr) { memcpy(client->response->buffer->output_ptr, (char *)at, length); client->response->buffer->output_ptr += length; + } else { + /* Do not cache body when http_on_body is called from esp_http_client_perform */ + if (client->state < HTTP_STATE_RES_ON_DATA_START && client->cache_data_in_fetch_hdr) { + ESP_LOGI(TAG, "Body received in fetch header state, %p, %d", at, length); + esp_http_buffer_t *res_buffer = client->response->buffer; + assert(res_buffer->orig_raw_data == res_buffer->raw_data); + res_buffer->orig_raw_data = (char *)realloc(res_buffer->orig_raw_data, res_buffer->raw_len + length); + if (!res_buffer->orig_raw_data) { + ESP_LOGE(TAG, "Failed to allocate memory for storing decoded data"); + return -1; + } + memcpy(res_buffer->orig_raw_data + res_buffer->raw_len, at, length); + res_buffer->raw_data = res_buffer->orig_raw_data; + } } client->response->data_process += length; @@ -713,6 +730,11 @@ esp_http_client_handle_t esp_http_client_init(const esp_http_client_config_t *co goto error; } + /* As default behavior, cache data received in fetch header state. This will be + * used in esp_http_client_read API only. For esp_http_perform we shall disable + * this as data will be processed by event handler */ + client->cache_data_in_fetch_hdr = 1; + client->parser_settings->on_message_begin = http_on_message_begin; client->parser_settings->on_url = http_on_url; client->parser_settings->on_status = http_on_status; @@ -754,6 +776,11 @@ esp_err_t esp_http_client_cleanup(esp_http_client_handle_t client) http_header_destroy(client->response->headers); if (client->response->buffer) { free(client->response->buffer->data); + if (client->response->buffer->orig_raw_data) { + free(client->response->buffer->orig_raw_data); + client->response->buffer->orig_raw_data = NULL; + client->response->buffer->raw_data = NULL; + } } free(client->response->buffer); free(client->response); @@ -940,7 +967,7 @@ esp_err_t esp_http_client_set_timeout_ms(esp_http_client_handle_t client, int ti static int esp_http_client_get_data(esp_http_client_handle_t client) { - if (client->state < HTTP_STATE_RES_COMPLETE_HEADER) { + if (client->state < HTTP_STATE_RES_ON_DATA_START) { return ESP_FAIL; } @@ -989,6 +1016,11 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len) res_buffer->raw_len -= remain_len; res_buffer->raw_data += remain_len; ridx = remain_len; + if (res_buffer->raw_len == 0) { + free(res_buffer->orig_raw_data); + res_buffer->orig_raw_data = NULL; + res_buffer->raw_data = NULL; + } } int need_read = len - ridx; bool is_data_remain = true; @@ -1089,10 +1121,16 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client) } /* falls through */ case HTTP_STATE_REQ_COMPLETE_DATA: + /* Disable caching response body, as data should + * be handled by application event handler */ + client->cache_data_in_fetch_hdr = 0; if (esp_http_client_fetch_headers(client) < 0) { if (client->is_async && errno == EAGAIN) { return ESP_ERR_HTTP_EAGAIN; } + /* Enable caching after error condition because next + * request could be performed using native APIs */ + client->cache_data_in_fetch_hdr = 1; if (esp_transport_get_errno(client->transport) == ENOTCONN) { ESP_LOGW(TAG, "Close connection due to FIN received"); esp_http_client_close(client); @@ -1103,7 +1141,10 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client) return ESP_ERR_HTTP_FETCH_HEADER; } /* falls through */ - case HTTP_STATE_RES_COMPLETE_HEADER: + case HTTP_STATE_RES_ON_DATA_START: + /* Enable caching after fetch headers state because next + * request could be performed using native APIs */ + client->cache_data_in_fetch_hdr = 1; if ((err = esp_http_check_response(client)) != ESP_OK) { ESP_LOGE(TAG, "Error response"); http_dispatch_event(client, HTTP_EVENT_ERROR, esp_transport_get_error_handle(client->transport), 0); @@ -1164,6 +1205,7 @@ int esp_http_client_fetch_headers(esp_http_client_handle_t client) } http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len); } + client->state = HTTP_STATE_RES_ON_DATA_START; ESP_LOGD(TAG, "content_length = %d", client->response->content_length); if (client->response->content_length <= 0) { client->response->is_chunked = true; @@ -1457,7 +1499,7 @@ void esp_http_client_add_auth(esp_http_client_handle_t client) if (client == NULL) { return; } - if (client->state != HTTP_STATE_RES_COMPLETE_HEADER) { + if (client->state != HTTP_STATE_RES_ON_DATA_START) { return; } if (client->redirect_counter >= client->max_authorization_retries) {