cesanta / mongoose

Embedded Web Server
https://mongoose.ws
Other
11.16k stars 2.73k forks source link

Fix multiple TLS records in buffer #2890

Closed scaprile closed 1 month ago

scaprile commented 2 months ago

Even though we receive IO_ERR, several outstanding TLS records can lie in our buffer. One or two extra calls is not enough to correctly process them and cleanly close.

NOTE: is_io_err becomes unused, will be removed further on if this proves to be the final battle.

fixes #2885 closes #2668

PS: In rare cases, rtls was filled with more than one record, and our capping strategy prevented us to call mg_tls_recv(). Fixed.

jcorporation commented 2 months ago

It seems to work more often than before, but not always. Attached a log with a failed download. log2.txt

scaprile commented 2 months ago

@jcorporation I can't reproduce this. With this patch I get 20 out of 20 correct downloads, 107586

jcorporation commented 2 months ago

Tried it again. Most times it works, but sometime it fails.

I freshly cloned latest master and merged the tls branch.

scaprile commented 2 months ago

I could reproduce it 1 in a 1000 tries... I'll add some debug helpers and see what I can do. Most likely we'll merge this patch (or equivalent) and try to isolate and fix the low failure rate bug later.

jcorporation commented 2 months ago

My failure rate is much higher. I tested with a wlan client. I will repeat my tests with lab and report back.

You tested with the url from my last tests?

scaprile commented 2 months ago

Yes, 10000 iterations 10 failures Maybe the problem is an incorrect response to a connection loss now ? I'll probably check tomorrow.

jcorporation commented 2 months ago

~I tested on a wired device and experienced the same failures regularly. I will check it with a different internet connection.~

I tested the same example with mongoose 7.14 and the error is gone. It seems to be a regression introduced in mongoose 7.15.

scaprile commented 1 month ago

@jcorporation I think I nailed it. Can't reproduce it on my side anymore, 0 out of 20000 downloads.

jcorporation commented 1 month ago

It works also for me. Thanks for your great support!