esp_https_ota doesn't follow redirects (IDFGH-850)

[olegantonyan]

esp_http_client_config_t config = {
    .url = url,
    .cert_pem = certs(),
    .max_redirection_count = 4,
    .disable_auto_redirect = false,
  };
  esp_err_t result = esp_https_ota(&config);

If url gets redirected OTA fails:
E (367424) esp_ota_ops: OTA image has invalid magic byte (expected 0xE9, saw 0x3c

0x3c == < ascii symbol

I expect redirects to work here because they work in esp_http_client and I need them to work because this is the way files storage work on my server.

[olegantonyan]

I have an idea why this happens. esp_http_client_read reads the first response, which was a redirect, we have to ignore it since it does not contain the data yet. The next response after redirect does, but we don't reach to it because this fails on the first read.

ota_write_err = esp_ota_write(update_handle, (const void *) upgrade_data_buf, data_read);
if (ota_write_err != ESP_OK) {  // upgrade_data_buf contains nonsense html from redirected response
    break;
}

I'm not sure if it's possible to skip first response read. This doesn't works:

while (1) {
        int status = esp_http_client_get_status_code(client);
        if (300 <= status && status < 599) {
          continue;
        }

        int data_read = esp_http_client_read(client, upgrade_data_buf, alloc_size);
        if (data_read == 0) {
            ESP_LOGI(TAG, "Connection closed, all data received");
            break;
        }

But here's my implementation with event instead of esp_http_client_read so I can check for response status on every read:

static bool ota_start(const char *url) {
  bool ok = false;

  esp_err_t err = ESP_OK;

  esp_http_client_handle_t client = NULL;

  do {
    ESP_LOGI(TAG, "starting ota");

    const esp_partition_t *update_partition = esp_ota_get_next_update_partition(NULL);
    if (update_partition == NULL) {
      ESP_LOGE(TAG, "passive ota partition not found");
      break;
    }
    ESP_LOGI(TAG, "writing to partition subtype %d at offset 0x%x", update_partition->subtype, update_partition->address);

    OTA_Context_t ctx;
    ctx.ok = true;
    ctx.handle = 0;

    if (esp_ota_begin(update_partition, OTA_SIZE_UNKNOWN, &ctx.handle) != ESP_OK) {
      ESP_LOGE(TAG, "esp_ota_begin failed: %s", esp_err_to_name(err));
      break;
    }
    ESP_LOGI(TAG, "esp_ota_begin succeeded");
    ESP_LOGI(TAG, "please wait...");

    esp_http_client_config_t config = {
      .event_handler = http_event_handle,
      .url = url,
      .timeout_ms = 6000,
      .method = HTTP_METHOD_GET,
      .cert_pem = certs(),
      .max_redirection_count = 4,
      .disable_auto_redirect = false,
      .user_data = (void *)&ctx,
      .buffer_size = 2048
    };
    ESP_LOGI(TAG, "request to %s", config.url);
    client = esp_http_client_init(&config);

    char ua[128] = { 0 };
    sysinfo_useragent(ua, sizeof(ua));
    esp_http_client_set_header(client, "User-Agent", ua);

    err = esp_http_client_perform(client);
    if (err == ESP_OK) {
      int status = esp_http_client_get_status_code(client);
      if (200 <= status && status < 300 && ctx.ok) {
        err = esp_ota_end(ctx.handle);
        if (err != ESP_OK) {
          ESP_LOGE(TAG, "esp_ota_end failed: %s", esp_err_to_name(err));
          break;
        }

        err = esp_ota_set_boot_partition(update_partition);
        if (err != ESP_OK) {
          ESP_LOGE(TAG, "esp_ota_set_boot_partition failed: %s", esp_err_to_name(err));
          break;
        }
        ESP_LOGI(TAG, "esp_ota_set_boot_partition succeeded");
        ok = true;
      }
    } else {
      ESP_LOGE(TAG, "http get request failed: %s", esp_err_to_name(err));
      break;
    }

  } while(false);

  esp_http_client_cleanup(client);

  return ok;
}

static esp_err_t http_event_handle(esp_http_client_event_t *evt) {
  switch(evt->event_id) {
    case HTTP_EVENT_ERROR:
      ESP_LOGD(TAG, "HTTP_EVENT_ERROR");
      break;
    case HTTP_EVENT_ON_CONNECTED:
      ESP_LOGD(TAG, "HTTP_EVENT_ON_CONNECTED");
      break;
    case HTTP_EVENT_HEADER_SENT:
      ESP_LOGD(TAG, "HTTP_EVENT_HEADER_SENT");
      break;
    case HTTP_EVENT_ON_HEADER:
      ESP_LOGD(TAG, "HTTP_EVENT_ON_HEADER %s = %s", evt->header_key, evt->header_value);
      break;
    case HTTP_EVENT_ON_DATA:
      ESP_LOGD(TAG, "HTTP_EVENT_ON_DATA, len=%d", evt->data_len);
      
      // Crucial part here! Don't read the response if it was redirect
      int status = esp_http_client_get_status_code(evt->client);
      if (300 <= status && status < 599) {
        break;
      }

      OTA_Context_t *ctx = (OTA_Context_t *)evt->user_data;
      esp_err_t ota_write_err = esp_ota_write(ctx->handle, evt->data, evt->data_len);
      if (ota_write_err != ESP_OK) {
        ESP_LOGE(TAG, "OTA write error: %s", esp_err_to_name(ota_write_err));
        ctx->ok = false;
      }
      break;
    case HTTP_EVENT_ON_FINISH:
      ESP_LOGD(TAG, "HTTP_EVENT_ON_FINISH");
      break;
    case HTTP_EVENT_DISCONNECTED:
      ESP_LOGD(TAG, "HTTP_EVENT_DISCONNECTED");
      break;
  }
  return ESP_OK;
}

[mahavirj]

@olegantonyan We have similar fix in our internal development queue, will try to prioritize and fix this soon. (CC @jitin17)

[jitin17]

@olegantonyan I am attaching a patch with support for URL redirection in esp_https_ota. Can you verify if this works?
esp_https_ota_redirect.tar.gz

[olegantonyan]

@jitin17 there's either a problem with the patch, or I'm too young to know how to use them :)

tar -xvf esp_https_ota_redirect.tar.gz
cd esp-idf/components/esp_https_ota/src

patch -p1 < ~/Desktop/esp_https_ota_redirect.patch 
patch unexpectedly ends in middle of line
patch: **** Only garbage was found in the patch input.

cat ~/Desktop/esp_https_ota_redirect.patch                                         
 �Om7��7��'(

Can you make a pull-request?

[jitin17]

It was a mistake from my side.
Can you try this one out:
esp_https_ota_redirect.tar.gz

[olegantonyan]

Yep, this seems to work

[ipapp]

When do you expect to merge this fix to master?

[MaazSk]

Hi I am facing the same issue.

[MaazSk]

Hi if anyone is still unable to perform OTA from google drive then please have a look here.

#8581

This patch works for me.

Thanks,
Maaz

[JRHemmen]

It looks like this patch from #8521 was included in v4.4 (#d485a8c), but I'm experiencing the same issue using IDF 5.2. Trying to download a binary attached to a github release redirects to objects.githubusercontent.com, which returns a 401. I'm able to successfully redirect and download in both my browser and POSTman.

ESP logs:
https://pastebin.com/NbxXhhtW

[mahavirj]

@JRHemmen

What happens if you increase the HTTP client buffer size (e.g., 2K)?

esp-idf/components/esp_http_client/include/esp_http_client.h

Line 164 in 0479494

int buffer_size; /*!< HTTP receive buffer size */

[JRHemmen]

@JRHemmen

What happens if you increase the HTTP client buffer size (e.g., 2K)?

esp-idf/components/esp_http_client/include/esp_http_client.h

Line 164 in 0479494

int buffer_size; /*!< HTTP receive buffer size */

Thanks for the quick reply. Just tried with a buffer size of 2048, 4096, and 8192 and still no luck. Though now it specifically errors for exceeding the buffer size.

https://pastebin.com/paNrwGfx

[mahavirj]

@JRHemmen

I was able to get the download working for the URL that you shared by applying following change to simple_ota example:

--- examples/system/ota/simple_ota_example/main/simple_ota_example.c
+++ examples/system/ota/simple_ota_example/main/simple_ota_example.c
@@ -98,6 +98,8 @@ void simple_ota_example_task(void *pvParameter)
 #endif /* CONFIG_EXAMPLE_USE_CERT_BUNDLE */
         .event_handler = _http_event_handler,
         .keep_alive_enable = true,
+        .buffer_size = 3000,
+        .buffer_size_tx = 3000,
 #ifdef CONFIG_EXAMPLE_FIRMWARE_UPGRADE_BIND_IF
         .if_name = &ifr,
 #endif

Redirection URL seems quite big and hence the change to buffer values. Please try once and see if this works.

[JRHemmen]

@mahavirj adding keep alive and the tx buffer resolved it. Thank you!