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

http client never finishes in asynchronous mode on HTTP_METHOD_HEAD (IDFGH-7444) #9020

Closed
MarcGeh opened this issue May 25, 2022 · 16 comments
Closed
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally

Comments

@MarcGeh
Copy link

MarcGeh commented May 25, 2022

Environment

  • Development Kit: ESP32-DevKitC
  • Kit version: v4
  • Module or chip used: ESP32-WROVER-E
  • IDF version: v4.4.1
  • Build System: idf.py
  • Compiler version: xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2-patch3) 8.4.0
  • Operating System: Linux
  • Using an IDE?: No
  • Power Supply: USB

Problem Description

When using the esp http client in asynchronous mode on a HTTPS connection with the method HTTP_METHOD_HEAD subsequent calls to esp_http_client_perform(..) will never return anything other than ESP_ERR_HTTP_EAGAIN if the connection has been established successfully and all headers have been received.

Expected Behavior

After all headers have been received esp_http_client_perform(..) should return ESP_OK just like in synchronous mode.

Actual Behavior

After all headers have been received esp_http_client_perform(..) will return ESP_ERR_HTTP_EAGAIN indefinitely.
I believe the issue is due to the behavior of esp_http_client_get_data line(..). In line 992 in esp_http_client.c this function returns 0 if the method is HTTP_METHOD_HEAD. While the client is in HTTP_STATE_RES_ON_DATA_START this function is called and since errno is not modified within it, it is most likely still set to EAGAIN from the previous transaction, which in turn causes this check in line 1181 to always be true.

Steps to reproduce

  1. Setup a http client for a HTTPS connection with is_async set to true
  2. Set the method to HTTP_METHOD_HEAD
  3. Call esp_http_client_perform in a loop
@espressif-bot espressif-bot added the Status: Opened Issue is new label May 25, 2022
@github-actions github-actions bot changed the title http client never finishes in asynchronous mode on HTTP_METHOD_HEAD http client never finishes in asynchronous mode on HTTP_METHOD_HEAD (IDFGH-7444) May 25, 2022
@ESP-YJM
Copy link
Collaborator

ESP-YJM commented May 26, 2022

@MarcGeh Thanks for your detailed description. Could you please add errno = 0 in this line

.

@MarcGeh
Copy link
Author

MarcGeh commented May 31, 2022

@ESP-YJM Thanks, I can confirm that this fixes the issue. Will this fix be part of the next release?

@ESP-YJM
Copy link
Collaborator

ESP-YJM commented May 31, 2022

@MarcGeh I'm not sure, but we will fix this as soon as possible.

@espressif-bot espressif-bot assigned ESP-YJM and unassigned mahavirj Jun 1, 2022
@MarcGeh
Copy link
Author

MarcGeh commented Jun 2, 2022

FYI for anyone having this issue and not wanting to modify any files inside the esp-idf to fix it, it is also sufficient to simply set errno = 0 before each call of esp_http_client_perform(...)

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jul 4, 2022
@espressif-bot espressif-bot added Status: Opened Issue is new and removed Status: In Progress Work is in progress labels Nov 7, 2022
@AxelLin
Copy link
Contributor

AxelLin commented Dec 17, 2022

@MarcGeh I'm not sure, but we will fix this as soon as possible.

@ESP-YJM How is the status now? Is this fixed?

@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Dec 19, 2022

@AxelLin Not yet, userspace can avoid it, so I lowered the priority.

@AxelLin
Copy link
Contributor

AxelLin commented Mar 19, 2023

@AxelLin Not yet, userspace can avoid it, so I lowered the priority.

Then this never get fixed.

@kriegste
Copy link

Any news here?

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Oct 18, 2023
@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Oct 18, 2023

@AxelLin @kriegste I have raise a MR internal.

@AxelLin
Copy link
Contributor

AxelLin commented Dec 3, 2023

@AxelLin @kriegste I have raise a MR internal.

How is the status now?

@AxelLin
Copy link
Contributor

AxelLin commented Dec 16, 2023

@MarcGeh I'm not sure, but we will fix this as soon as possible.

@ESP-YJM ASAP? I'm wondering if this will be fixed in 2023.

@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Dec 18, 2023

@AxelLin Sorry to response late. The MR need add some automated test and i am busy for other projects. When I have time I will add and push this MR integration. I think this problem can be avoided at the application layer first.

@AxelLin
Copy link
Contributor

AxelLin commented Dec 18, 2023

@ESP-YJM

Just remind this issue was reported on May 25, 2022, which is quite long time ago.
Seems you think people can easily workaround the issue.
However, for most users it's not easy to find this old issue at all.
Which means it may take quite long time to figure out why it does not work.
IMHO, you should fix it asap and backport to release branches since this is indeed a BUG.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Jan 8, 2024
@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Jan 8, 2024

@AxelLin It has beed merged into master branch and it will backport to v5.x branch.

@ESP-YJM ESP-YJM closed this as completed Jan 8, 2024
@AxelLin
Copy link
Contributor

AxelLin commented Jan 8, 2024

@ESP-YJM Could you share which commit fixed this?

@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Jan 8, 2024

@AxelLin It has merged into the internal master branch and need wait ci passed and will sync to github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

6 participants