feedc0de/esp-idf
1
0
Fork 0
forked from espressif/esp-idf
Code Pull Requests Activity

esp_http_client/esp_https_ota: Removed errno checks

Browse Source 
- Returned -ESP_ERR_HTTP_EAGAIN for timeout errors from
  esp_http_client whenever tcp transport layer returns
  connection timeout
- Removed redundant conditional statements as required

Co-authored-by: Shubham Kulkarni <shubham.kulkarni@espressif.com>
This commit is contained in:
Laukik Hase
2022-03-07 11:05:43 +05:30
parent cc7c67ad4e
commit 7115881a97
3 changed files with 37 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
components/esp_http_client/esp_http_client.c
View File

@@ -1063,9 +1063,8 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len)
if (rlen <= 0) { if (rlen <= 0) {
if (errno != 0) { if (errno != 0) {
esp_log_level_t sev = ESP_LOG_WARN; esp_log_level_t sev = ESP_LOG_WARN;
/* On connection close from server, recv should ideally return 0 but we have error conversion /* Check for cleanly closed connection */
* in `tcp_transport` SSL layer which translates it `-1` and hence below additional checks */ if (rlen == ERR_TCP_TRANSPORT_CONNECTION_CLOSED_BY_FIN && client->response->is_chunked) {
if (rlen == -1 && errno == ENOTCONN && client->response->is_chunked) {
/* Explicit call to parser for invoking `message_complete` callback */ /* Explicit call to parser for invoking `message_complete` callback */
http_parser_execute(client->parser, client->parser_settings, res_buffer->data, 0); http_parser_execute(client->parser, client->parser_settings, res_buffer->data, 0);
/* ...and lowering the message severity, as closed connection from server side is expected in chunked transport */ /* ...and lowering the message severity, as closed connection from server side is expected in chunked transport */
@@ -1073,14 +1072,21 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len)
} }
ESP_LOG_LEVEL(sev, TAG, "esp_transport_read returned:%d and errno:%d ", rlen, errno); ESP_LOG_LEVEL(sev, TAG, "esp_transport_read returned:%d and errno:%d ", rlen, errno);
} }
#ifdef CONFIG_ESP_HTTP_CLIENT_ENABLE_HTTPS
if (rlen == ESP_TLS_ERR_SSL_WANT_READ || errno == EAGAIN) { if (rlen == ERR_TCP_TRANSPORT_CONNECTION_TIMEOUT) {
#else ESP_LOGD(TAG, "Connection timed out before data was ready!");
if (errno == EAGAIN) { /* Returning the number of bytes read upto the point where connection timed out */
#endif if (ridx) {
ESP_LOGD(TAG, "Received EAGAIN! rlen = %d, errno %d", rlen, errno);
return ridx; return ridx;
} }
return -ESP_ERR_HTTP_EAGAIN;
}
if (rlen != ERR_TCP_TRANSPORT_CONNECTION_CLOSED_BY_FIN) {
esp_err_t err = esp_transport_translate_error(rlen);
ESP_LOGE(TAG, "transport_read: error - %d | %s", err, esp_err_to_name(err));
}
if (rlen < 0 && ridx == 0 && !esp_http_client_is_complete_data_received(client)) { if (rlen < 0 && ridx == 0 && !esp_http_client_is_complete_data_received(client)) {
http_dispatch_event(client, HTTP_EVENT_ERROR, esp_transport_get_error_handle(client->transport), 0); http_dispatch_event(client, HTTP_EVENT_ERROR, esp_transport_get_error_handle(client->transport), 0);
return ESP_FAIL; return ESP_FAIL;
@@ -1142,8 +1148,9 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client)
/* Disable caching response body, as data should /* Disable caching response body, as data should
* be handled by application event handler */ * be handled by application event handler */
client->cache_data_in_fetch_hdr = 0; client->cache_data_in_fetch_hdr = 0;
if (esp_http_client_fetch_headers(client) < 0) { int64_t ret = esp_http_client_fetch_headers(client);
if (client->is_async && errno == EAGAIN) { if (ret < 0) {
if ((client->is_async && errno == EAGAIN) || ret == -ESP_ERR_HTTP_EAGAIN) {
return ESP_ERR_HTTP_EAGAIN; return ESP_ERR_HTTP_EAGAIN;
} }
/* Enable caching after error condition because next /* Enable caching after error condition because next
@@ -1219,6 +1226,10 @@ int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client)
while (client->state < HTTP_STATE_RES_COMPLETE_HEADER) { while (client->state < HTTP_STATE_RES_COMPLETE_HEADER) {
buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size_rx, client->timeout_ms); buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size_rx, client->timeout_ms);
if (buffer->len <= 0) { if (buffer->len <= 0) {
if (buffer->len == ERR_TCP_TRANSPORT_CONNECTION_TIMEOUT) {
ESP_LOGW(TAG, "Connection timed out before data was ready!");
return -ESP_ERR_HTTP_EAGAIN;
}
return ESP_FAIL; return ESP_FAIL;
} }
http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len); http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len);

3
components/esp_http_client/include/esp_http_client.h
View File

@@ -427,6 +427,7 @@ int esp_http_client_write(esp_http_client_handle_t client, const char *buffer, i
* @return * @return
* - (0) if stream doesn't contain content-length header, or chunked encoding (checked by `esp_http_client_is_chunked` response) * - (0) if stream doesn't contain content-length header, or chunked encoding (checked by `esp_http_client_is_chunked` response)
* - (-1: ESP_FAIL) if any errors * - (-1: ESP_FAIL) if any errors
* - (-ESP_ERR_HTTP_EAGAIN = -0x7007) if call is timed-out before any data was ready
* - Download data length defined by content-length header * - Download data length defined by content-length header
*/ */
int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client); int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client);
@@ -451,6 +452,8 @@ bool esp_http_client_is_chunked_response(esp_http_client_handle_t client);
* @return * @return
* - (-1) if any errors * - (-1) if any errors
* - Length of data was read * - Length of data was read
*
* @note (-ESP_ERR_HTTP_EAGAIN = -0x7007) is returned when call is timed-out before any data was ready
*/ */
int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len); int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len);

28
components/esp_https_ota/src/esp_https_ota.c
View File

@@ -361,11 +361,11 @@ static esp_err_t read_header(esp_https_ota_t *handle)
data_read = esp_http_client_read(handle->http_client, data_read = esp_http_client_read(handle->http_client,
(handle->ota_upgrade_buf + bytes_read), (handle->ota_upgrade_buf + bytes_read),
data_read_size); data_read_size);
/* if (data_read < 0) {
* As esp_http_client_read doesn't return negative error code if select fails, we rely on if (data_read == -ESP_ERR_HTTP_EAGAIN) {
* `errno` to check for underlying transport connectivity closure if any ESP_LOGD(TAG, "ESP_ERR_HTTP_EAGAIN invoked: Call timed out before data was ready");
*/ continue;
if (errno == ENOTCONN || errno == ECONNRESET || errno == ECONNABORTED || data_read < 0) { }
ESP_LOGE(TAG, "Connection closed, errno = %d", errno); ESP_LOGE(TAG, "Connection closed, errno = %d", errno);
break; break;
} }
@@ -491,19 +491,9 @@ esp_err_t esp_https_ota_perform(esp_https_ota_handle_t https_ota_handle)
* esp_http_client_is_complete_data_received is added to check whether * esp_http_client_is_complete_data_received is added to check whether
* complete image is received. * complete image is received.
*/ */
bool is_recv_complete = esp_http_client_is_complete_data_received(handle->http_client); if (!esp_http_client_is_complete_data_received(handle->http_client)) {
/* ESP_LOGE(TAG, "Connection closed before complete data was received!");
* As esp_http_client_read doesn't return negative error code if select fails, we rely on
* `errno` to check for underlying transport connectivity closure if any.
* Incase the complete data has not been received but the server has sent
* an ENOTCONN or ECONNRESET, failure is returned. We close with success
* if complete data has been received.
*/
if ((errno == ENOTCONN || errno == ECONNRESET || errno == ECONNABORTED) && !is_recv_complete) {
ESP_LOGE(TAG, "Connection closed, errno = %d", errno);
return ESP_FAIL; return ESP_FAIL;
} else if (!is_recv_complete) {
return ESP_ERR_HTTPS_OTA_IN_PROGRESS;
} }
ESP_LOGD(TAG, "Connection closed"); ESP_LOGD(TAG, "Connection closed");
} else if (data_read > 0) { } else if (data_read > 0) {
@@ -523,6 +513,10 @@ esp_err_t esp_https_ota_perform(esp_https_ota_handle_t https_ota_handle)
#endif // CONFIG_ESP_HTTPS_OTA_DECRYPT_CB #endif // CONFIG_ESP_HTTPS_OTA_DECRYPT_CB
return _ota_write(handle, data_buf, data_len); return _ota_write(handle, data_buf, data_len);
} else { } else {
if (data_read == -ESP_ERR_HTTP_EAGAIN) {
ESP_LOGD(TAG, "ESP_ERR_HTTP_EAGAIN invoked: Call timed out before data was ready");
return ESP_ERR_HTTPS_OTA_IN_PROGRESS;
}
ESP_LOGE(TAG, "data read %d, errno %d", data_read, errno); ESP_LOGE(TAG, "data read %d, errno %d", data_read, errno);
return ESP_FAIL; return ESP_FAIL;
} }