esp_https_ota keeps trying to connect after max_authorization_retries
Posted: Thu Mar 16, 2023 10:17 pm
I'm using https OTA with basic authentication and max_authorization_retries set to -1. After trying to connect to the server with an incorrect password, esp_https_ota_begin keeps attempting to connect to the server even though I get an error which says "HTTP_CLIENT: Error, reached max_authorization_retries count=0". The continued login attempts result in my server host blocking my IP address for too many failed login attempts.
I am using esp-idf-v5.0.1 with partial downloads disabled.
As far as I can tell, the problem is in _http_connect on esp_https_ota.c because process_again always returns true for the case where a 401 status (HttpStatus_Unauthorized) has been received from the server no matter what value has been set for max_authorization_retries so we never jump out of the loop in _http_connect.
Shouldn't _http_connect jump out of the loop and return an error if max_authorization_retries has been exceeded?
I was able to make it work by adding a function called "esp_http_client_n_retries_exceeded" in esp_http_client.c and then modifying _http_connect in esp_https_ota.c to check it when it checks process_again at the end of the loop. It will fail immediately for the case where max_authorization_retries has been set to 0 or -1 even if the username and password for basic authentication are correct.
If there is a fix that doesn't involve modifying the esp-idf I would love to know. I could hook into http header events and check for 401 status but that seems like a clunky option. This also doesn't work for the case where partial downloads are enabled.
I am using esp-idf-v5.0.1 with partial downloads disabled.
As far as I can tell, the problem is in _http_connect on esp_https_ota.c because process_again always returns true for the case where a 401 status (HttpStatus_Unauthorized) has been received from the server no matter what value has been set for max_authorization_retries so we never jump out of the loop in _http_connect.
- static esp_err_t _http_connect(esp_http_client_handle_t http_client)
- {
- esp_err_t err = ESP_FAIL;
- int status_code, header_ret;
- do {
- char *post_data = NULL;
- /* Send POST request if body is set.
- * Note: Sending POST request is not supported if partial_http_download
- * is enabled
- */
- int post_len = esp_http_client_get_post_field(http_client, &post_data);
- err = esp_http_client_open(http_client, post_len);
- if (err != ESP_OK) {
- ESP_LOGE(TAG, "Failed to open HTTP connection: %s", esp_err_to_name(err));
- return err;
- }
- if (post_len) {
- int write_len = 0;
- while (post_len > 0) {
- write_len = esp_http_client_write(http_client, post_data, post_len);
- if (write_len < 0) {
- ESP_LOGE(TAG, "Write failed");
- return ESP_FAIL;
- }
- post_len -= write_len;
- post_data += write_len;
- }
- }
- header_ret = esp_http_client_fetch_headers(http_client);
- if (header_ret < 0) {
- return header_ret;
- }
- status_code = esp_http_client_get_status_code(http_client);
- err = _http_handle_response_code(http_client, status_code);
- if (err != ESP_OK) {
- return err;
- }
- } while (process_again(status_code));
- return err;
- }
- static bool process_again(int status_code)
- {
- switch (status_code) {
- case HttpStatus_MovedPermanently:
- case HttpStatus_Found:
- case HttpStatus_SeeOther:
- case HttpStatus_TemporaryRedirect:
- case HttpStatus_PermanentRedirect:
- case HttpStatus_Unauthorized:
- return true;
- default:
- return false;
- }
- return false;
- }
I was able to make it work by adding a function called "esp_http_client_n_retries_exceeded" in esp_http_client.c and then modifying _http_connect in esp_https_ota.c to check it when it checks process_again at the end of the loop. It will fail immediately for the case where max_authorization_retries has been set to 0 or -1 even if the username and password for basic authentication are correct.
If there is a fix that doesn't involve modifying the esp-idf I would love to know. I could hook into http header events and check for 401 status but that seems like a clunky option. This also doesn't work for the case where partial downloads are enabled.
- bool esp_http_client_n_retries_exceeded(esp_http_client_handle_t client)
- {
- if (client->redirect_counter >= client->max_authorization_retries)
- {
- return true;
- }
- return false;
- }
- static esp_err_t _http_connect(esp_http_client_handle_t http_client)
- {
- esp_err_t err = ESP_FAIL;
- int status_code, header_ret;
- do {
- char *post_data = NULL;
- /* Send POST request if body is set.
- * Note: Sending POST request is not supported if partial_http_download
- * is enabled
- */
- int post_len = esp_http_client_get_post_field(http_client, &post_data);
- err = esp_http_client_open(http_client, post_len);
- if (err != ESP_OK) {
- ESP_LOGE(TAG, "Failed to open HTTP connection: %s", esp_err_to_name(err));
- return err;
- }
- if (post_len) {
- int write_len = 0;
- while (post_len > 0) {
- write_len = esp_http_client_write(http_client, post_data, post_len);
- if (write_len < 0) {
- ESP_LOGE(TAG, "Write failed");
- return ESP_FAIL;
- }
- post_len -= write_len;
- post_data += write_len;
- }
- }
- header_ret = esp_http_client_fetch_headers(http_client);
- if (header_ret < 0) {
- return header_ret;
- }
- status_code = esp_http_client_get_status_code(http_client);
- err = _http_handle_response_code(http_client, status_code);
- if (err != ESP_OK) {
- return err;
- }
- } while (process_again(status_code) && !esp_http_client_n_retries_exceeded(http_client));
- // if we exceeded our number of authentication retries return an error
- if (esp_http_client_n_retries_exceeded(http_client))
- {
- ESP_LOGE(TAG,"n retries exceeded");
- err = ESP_FAIL;
- }
- return err;
- }