QuickSander / ArduinoHttpServer

Server side minimalistic HTTP protocol implementation for the Arduino platform.
GNU General Public License v3.0
23 stars 11 forks source link

Add basic authorization support. #16

Closed tomer-w closed 3 years ago

tomer-w commented 3 years ago

Hi QuickSander,

I really like this HTTP server, noticeably light. I was missing the basic authorization support so decided to add it. There is only one caveat with this change, it does require dependency on another library to get base64 encoding which is needed for this task. I'm not sure I know how to mark that dependency so the Arduino IDE will be aware of that. That library is tiny so I hope it will be ok with you.

I also did some minor fix in your sample as it seems you are missing closing the connection.

If you are fine with that direction, I can add new sample to show the usage.

Thanks, Tomer.

tomer-w commented 3 years ago

I think I fixed all comments. Let me know if anything is missing.

tomer-w commented 3 years ago

Tnx!!

QuickSander commented 3 years ago

Thank you. If you don't mind I will tweak it a bit to attempt to reduce the String usage and change it into FixString. I prefer the deterministic behaviour FixString offers (no surprise Arduino suddenly lock up since some allocation fails due to memory fragmentation)

tomer-w commented 3 years ago

Sure. Good luck. I dont think there is a way to do that without limiting the username and password length, but I might be wrong. As the malloc and free happen in the same function, I dont think it can create fragmentation but I might be wrong here as well. I think it will happen only if you have them long lived and than deleted. Also the check we have there will just fail the login and should not crash the device.

QuickSander commented 3 years ago

Agree with your rationale. Nevertheless I prefer static allocation on embedded devices as the compiler can better optimise design time known allocation. It all depends on the situation, but I prefer here to use FixString whenever possible (in attempt to get it down to zero heap allocation which takes a tiny bit more clock cycles). Although I did not convert everything yet to FixString myself. This does come indeed with the downside that credentials have a maximum size. But arduino itself has limited memory anyway.

Thanks again.

tomer-w commented 3 years ago

If we are heading to that direction, I think an important candidate to improve is ArduinoHttpServer::AbstractStreamHttpReply::send(). I dont use that function as i'm low on memory and that one doing lot of string copying which is bad for big replies. Did you consider just getting the body as char* and instead of the copy to call to the getStream().print multiple times? This is what I am doing in my project and it works very well. Is there any major benefit of calling it once and paying with the buffer manipulation?

Thanks, Tomer.