ArduinoWebsockets icon indicating copy to clipboard operation
ArduinoWebsockets copied to clipboard

Supporting case-insensitive headers

Open khoih-prog opened this issue 2 years ago • 2 comments

Supporting case-insensitive headers, according to RFC2616, as requested in Make headers case-insensitive #105

khoih-prog avatar Dec 19 '21 01:12 khoih-prog

Hi!

First of all, I really appreciate the code donation and the time you invested in it.

Although this is an important feature (actually bugfix...) the changes seem massive compared to the functionality change, and because I'm maintaining this library with minimal time available I can't thoroughly test your changes. Also, I think a much leaner wrapper around string comparison (like strcasecmp) would suffice here.

I would:

  1. Skip the consts, as I don't really think it makes the code more readable
  2. Not make case insensitive support optional
  3. Instead of splitting headers to headers and lowheaders, I would define an std::map with a custom comperator that would internally use case insensitive comparison, and will probably save most of the need for tolower calls and so on

Again, thank you for the contribution, but I don't think it matches my code style. Also the Origin default is your khoih-prog/Websockets2_Generic now :p

BTW, I was really excited to see your websockets library! I'm so glad my library was useful for starting your own, and I really appreciate the credits in the README and code!

Best wishes, Gil.

gilmaimon avatar Dec 24 '21 12:12 gilmaimon

Sorry for late reply.

There are some unintentional left-over mistakes when I copied / edited for the PR.

Not make case insensitive support optional

I make it optional just to make Client optionally sending the lower-case headers to test the fixed case-insensitive Server

I'll have a look, fix and update according to your recommendation soon.

Best Regards,

khoih-prog avatar Dec 30 '21 22:12 khoih-prog