Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"esp_flash_api.c" and "transport_ssl.c“ make some changes (IDFGH-2486) #4598

Closed
xiruilin opened this issue Jan 8, 2020 · 6 comments
Closed
Labels
Type: Bug bugs in IDF

Comments

@xiruilin
Copy link

xiruilin commented Jan 8, 2020

Environment

  • Development Kit: [ESP32-LyraT]
  • Module or chip used: [ESP32-WROVER]
  • IDF version (): v4.1-dev-1770-g71b4768df
  • Build System: [idf.py]
  • Compiler version (): xtensa-esp32-elf-gcc (crosstool-NG esp-2019r2) 8.2.0
  • Operating System: [Windows]
  • (Windows only) environment type: [ESP Command Prompt].
  • Using an IDE?: [No]
  • Power Supply: [USB]

Problem Description

In esp_flash_api.c lines 501, function esp_flash_read(), if read length too big, then
length_to_allocate = MAX(MAX_READ_CHUNK, length) will cause memory allocation to fail.
In lines 519, length_to_read = MIN(MAX_READ_CHUNK, length), so it can be modified as follows:

diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c
index ff9c49e4d..a91a05775 100644
--- a/components/spi_flash/esp_flash_api.c
+++ b/components/spi_flash/esp_flash_api.c
@@ -498,7 +498,7 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
     uint8_t* temp_buffer = NULL;
 
     if (!direct_read) {
-        uint32_t length_to_allocate = MAX(MAX_READ_CHUNK, length);
+        uint32_t length_to_allocate = MIN(MAX_READ_CHUNK, length);
         length_to_allocate = (length_to_allocate+3)&(~3);
         temp_buffer = heap_caps_malloc(length_to_allocate, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
         ESP_LOGV(TAG, "allocate temp buffer: %p", temp_buffer);

In transport_ssl.c lines 168, function ssl_close(), if https request got 301 or 302 status code,
need to redirect, the ssl state must be reset to origin, changed below:

diff --git a/components/tcp_transport/transport_ssl.c b/components/tcp_transport/transport_ssl.c
index acc963666..5501b2383 100644
--- a/components/tcp_transport/transport_ssl.c
+++ b/components/tcp_transport/transport_ssl.c
@@ -172,6 +172,7 @@ static int ssl_close(esp_transport_handle_t t)
     if (ssl->ssl_initialized) {
         esp_tls_conn_delete(ssl->tls);
         ssl->ssl_initialized = false;
+        ssl->conn_state = TRANS_SSL_INIT;
     }
     return ret;
 }

Thanks!

@xiruilin xiruilin added the Type: Bug bugs in IDF label Jan 8, 2020
@github-actions github-actions bot changed the title "esp_flash_api.c" and "transport_ssl.c“ make some changes "esp_flash_api.c" and "transport_ssl.c“ make some changes (IDFGH-2486) Jan 8, 2020
@Alvin1Zhang
Copy link
Collaborator

@xiruilin Thanks for reporting, we would look into. Thanks.

@ginkgm
Copy link
Collaborator

ginkgm commented Apr 8, 2020

Hi @xiruilin ,

The first question seems to be solved in bfc37ab. Could you have a try?

@mahavirj
Copy link
Member

mahavirj commented Apr 9, 2020

@xiruilin For the transport_ssl change suggested above, do you have test case or code snippet which fails to work without this?

@xiruilin
Copy link
Author

@ginkgm
The first problem has been solved, thanks!

@Alvin1Zhang
Copy link
Collaborator

@xiruilin Glad to hear the first issue has been resolved, how about the second issue? For the transport_ssl change suggested above, do you have test case or code snippet which fails to work without this? Thanks.

@xiruilin
Copy link
Author

@Alvin1Zhang @mahavirj
Sorry for my late answer.
The second issue occurred on an asynchronous connection, if not reset
ssl->conn_state to TRANS_SSL_INIT in ssl_close(),
esp_http_client_set_redirection() would got the issue.
In file "/esp-idf/components/tcp_transport/transport_ssl.c" lines 48,
function ssl_connect_async() check the ssl->conn_state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

4 participants