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

5.2 beta 2 : esp_http_client bug is back (IDFGH-12032) #13097

Closed
3 tasks done
dannybackx opened this issue Feb 3, 2024 · 8 comments
Closed
3 tasks done

5.2 beta 2 : esp_http_client bug is back (IDFGH-12032) #13097

dannybackx opened this issue Feb 3, 2024 · 8 comments
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally

Comments

@dannybackx
Copy link

dannybackx commented Feb 3, 2024

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

There used to be a bug in esp_http_client which caused it to cut headers in two if they would arrive in separate chunks. So the application would receive only piece of the actual value of a variable. It's possible to work around it by playing with buffer size, but not reliably (depends on what you get back, which happens to be one of the things an ACME server deliberately plays with).

Example of what this does to my example program :

% fgrep -i -a setNonce typescript.staging.19
E (17:37:53.970) Acme: setNonce(7K8fIQp7225wX2yeYmb_1wC-0e1FWJxgadVwbWzfdSN-3TDtGjc)
E (17:37:56.129) Acme: setNonce(VlC4gE5VhyEbtxX239pBytgxaZB6Pclfh_8kaicsZyesbe_XyFo)
E (17:37:58.014) Acme: setNonce(7K8fIQp7lMijNXvbVKHxaj)
E (17:37:59.819) Acme: setNonce(7K8fIQp7V7cGKGfODp5cdTXp-Ic4x5YWunkWff18K9p764sNeMQ)
E (17:38:11.888) Acme: setNonce(VlC4gE5VoPaXOlQ7a3-ArQg9QaaZUrQWFFoMh-IUGpiMaldAkzU)
E (17:38:23.974) Acme: setNonce(7K8fIQp776wlNTWzZ9ZJy8ooF-lrG9ZyNEjCflz95kElhCcmxxM)
E (17:38:25.944) Acme: setNonce(VlC4gE5VXhiwCEIaO8PIROJe)
E (17:38:38.189) Acme: setNonce(VlC4gE5V02NMASDCTacASOT30XIjed6bRxlELEpSqtrGhMP9-hQ)
E (17:38:50.568) Acme: setNonce(7K8fIQp7Z8ZlprcglZRqd0qVOZlqLp1ZucNjYcOr4jS-JRgYPr8)

Can be demonstrated by the "standalone" example in my acmeclient library.
See svn+ssh://dannybackx@svn.code.sf.net/p/esp32-acme-client/code or https://components.espressif.com/components/dannybackx/acmeclient .

Note that my code appears to recover from it, but that is probably not so for other applications.

See also #7157 .

@espressif-bot espressif-bot added the Status: Opened Issue is new label Feb 3, 2024
@github-actions github-actions bot changed the title 5.2 beta 2 : esp_http_client bug is back 5.2 beta 2 : esp_http_client bug is back (IDFGH-12032) Feb 3, 2024
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Feb 28, 2024
@AxelLin
Copy link
Contributor

AxelLin commented Mar 19, 2024

@hmalpani Any update?

@hmalpani
Copy link
Contributor

@dannybackx
Thanks for reporting the issue. Apologies for the delayed response. I am unable to reproduce the issue. Can you share a minimal code which can be used to reproduce this? It will be helpful in debugging the issue. Also, can you try reverting this commit and check if it helps resolve the issue: 04ac8e4. Although I am not sure if it is the cause of the issue. Please let me know the observations.

@dannybackx
Copy link
Author

Reproducing this is actually not very hard.

I currently have a workaround in my code like this :
// Work around esp-idf esp_http_client bug that reappeared
httpc.buffer_size = 2048;

This works for now until I get bigger results back from the server.

But in this, there also lies the way to reproduce. So use a simplistic client example, and add a line liek the one above but with a ridiculously low buffer size (e.g. 25).

When querying e.g. http://www.google.com I currently get (partial output from curl -v) :
< Content-Type: text/html; charset=ISO-8859-1
< Content-Security-Policy-Report-Only: object-src 'none';base-uri 'self';script-src 'nonce-F6TW0J50XtIUapNJhjVLlQ' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp

You should observe that with a low buffer size, these lines are not treated well by the current source, because it will break up the long lines so you get invalid (partial) content instead of the full headers.

@hmalpani
Copy link
Contributor

hmalpani commented Mar 20, 2024

Hello @dannybackx
I have verified that reverting the commit: 04ac8e4 fix the issue. I will create an internal MR to fix this issue and it should be available on GitHub once it merges internally. Until then you can try reverting the commit. Hope it helps.

@AxelLin
Copy link
Contributor

AxelLin commented Apr 1, 2024

@hmalpani Could you share what issue was fixed by commit 04ac8e4 ?

espressif-bot pushed a commit that referenced this issue Apr 3, 2024
@AxelLin
Copy link
Contributor

AxelLin commented Apr 8, 2024

@hmalpani
Still needs fix for v5.1, v5.0, v4.4.

espressif-bot pushed a commit that referenced this issue Apr 12, 2024
@AxelLin
Copy link
Contributor

AxelLin commented Apr 16, 2024

@hmalpani v5.1 still needs fix, the backport fix is so slow.

@hmalpani
Copy link
Contributor

Hello @AxelLin
We have backported the fix to v5.1. It will be available soon on GitHub.

@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 Apr 16, 2024
espressif-bot pushed a commit that referenced this issue Apr 17, 2024
espressif-bot pushed a commit that referenced this issue Apr 17, 2024
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

4 participants