Add content-length check for response (to fix http client) - Githubissues

chunkysteveo / OctoPrintAPI

Library for use with Arduino compatible micro controllers (web enabled) to access the Octoprint API on Raspberry Pi's running the Octoprint 3D printer web server by the brilliant Gina Häußge, aka @foosel.

GNU General Public License v2.0

42 stars 24 forks source link

Add content-length check for response (to fix http client) #11

Closed sidddy closed 5 years ago

sidddy commented 6 years ago

Another attempt to fix the http client of OctoprintAPI :-)

switched back to "Connection: keep-alive" to avoid weird issues with new esp8266 core
added handling of Content-length header in response, read only the advertised number of bytes from the response stream (avoids waiting for the three second timeout for every request)
use a generic request method for both GET and POST requests to reduce code duplication
my editor did some trailing whitespace cleanups... I hope you don't mind :-)

sidddy commented 6 years ago

ah, fixes #8 btw

chunkysteveo commented 6 years ago

Thanks @sidddy ! Let me take a look at the updates and see how well they work. Will dig out my ESp32 boards too to make sure they work too. Like the idea of the "request" to reduce code duplication for GET and POST, nice.

chunkysteveo commented 5 years ago

Going to merge this one @sidddy and work from the master to make an updated version for Arduino Library Manager. Thanks for your continued efforts!

chunkysteveo commented 5 years ago

Do you have an ESP32 @sidddy ? The latest updates to the library are not working with these boards. Am trying to diagnose and fix, but if you have an ESP32, may be worth you checking out the HelloWorld example to see if you can spot anything.

sidddy commented 5 years ago

yes, I can have a look, probably tomorrow.

chunkysteveo commented 5 years ago

No problem, no rush. Am having a play myself, but many eyes make light work of spotting things!! thanks.

sidddy commented 5 years ago

I think I found the problem: Somehow, the user agent name became too long for the 40 charracter array which was supposed to store it... I enlarged the array and switched to snprintf instead of sprintf to avoid such problems in the future. see #14

chunkysteveo commented 5 years ago

Son of a....! That was a line I added, damn it!! But well found @sidddy, i will test on my ESP32 tomorrow. Out of interest, how did you spot that, or did you just count up the length and check?

sidddy commented 5 years ago

The esp32 detected that there's a stack corruption, so the error had to be caused by writing too much data to some local variable. Unfortunately, the stack dump of the exception is not useful in such case (because... well... the stack is corrupted ;-) ), so I had to add some ugly debug statements to find out in which method it happens and then manually look in that method for candidates.

chunkysteveo commented 5 years ago

Wow, it works! That's great, thanks for the prompt update! I realised I miss counted the define string for the variable, but had no real way of troubleshooting that to get to it. Reading up on snprintf too now - have so much to learn still!! Tested it with a buffer size of 41 and it works too - but going to keep your 64 length. I did think that the user agent would never grow in size from version to the next, but obviously 1.1.10 is longer than 1.1.9, crap! Interesting that the ESP8266 didn't seem to bother with this issue?

thanks again, going to pull in your request!

sidddy commented 5 years ago

My pleasure, I'm happy that it works for you as well! I guess it just luck that it worked for esp8266: It was just overflowing by a single byte, so it might have worked due to padding (i.e. a few bytes of unallocated memory after the useragent variable).

chunkysteveo commented 5 years ago

Ahh yes, it was a doofus mistake by me to not actually double check the length of something!! "that's what she said!!" All good now, thanks again, off to learn more C I think!