Fix integer length overflow in basic auth

[dirkmueller]

The string lenths were added and then stored in "char" which is
typically signed and limited to values up to 127. Using size_t
is a more appropriate type.

Simplify the code a bit as well to avoid the code smell.

[devyte]

Don't do this, at least not in a header file. This imports everything in esp8266webserver into the global namespace, which defeats the purpose of using a namespace.
It is sometimes ok to do this in a cpp file, but even then it's better to import into the cpp's anonymous namespace and not into the global one.

[dirkmueller]

I am actually not able to spot something that is outside the namespace in "nm" symbol dump. can you elaborate an example? the using doesn't change namespace, it just avoid me having to put a quadrillion esp8266webserver:: prefixes to all the declarations..

I can do that if your concern is the "using" part.

[dirkmueller]

Although I'm not sure I understood the concern, because there are no globals only statics (which are not exported hence don't pollute and don't care in which namespace they're declared), I moved all #include's out of the -impl.h now and have my duplicate-import problem solved this way.

[devyte]

These two were inside the namespace because their implementations are not in a namespace. Pulling them out like this means that the internals are moved to the global namespace, which is undesirable.

[dirkmueller]

the issue is that the things that they #include'ed was now being put into that namespace, which causes duplicate instanciations (code bloat) or missing symbols..

[devyte]

You're correct about the last part. In general, it is bad practice to include inside a namespace for this exact reason.
The correct way to handle this is to separate the headers, so a bit more work along the lines of the following. Are you game?

1. ESP8266WebServer-impl.h:
   • Put namespace esp8266webserver around all the code in there.
   • In ESP8266WebServer.h move the include to after the namespace closing like you had previously proposed.
   • Move the three headers added to ESP8266WebServer.h in your latest proposal back to the top of ESP8266WebServer-impl.h.
2. RequestHandler.h: Wrapping with a namespace here could cause havoc in multiple dimensions. To avoid that:
   • Fully qualify the template import like so: using WebServerType = esp8266webserver::ESP8266WebServerTemplate<ServerType>;
3. RequestHandlersImpl.h:
   • Keep your removal of using namespace mime in the global namespace. and its use where specifically used
   • Qualify the using WebServerType same way as above (x2)
4. Move the above includes out of the esp8266 namespace as you had proposed.

[dirkmueller]

I can do that cleanup. does it belong into this PR? I would prefer it as a followup.

[devyte]

Well, I proposed the above as part of this PR because you started the include changes here. If you prefer to separate, what you could do is keep just the integer overflow fix for the ::authenticate() method here, and do the include cleanup in a different PR. What do you think?

[devyte]

A side note:
Whether char is signed or unsigned is implementation specific. In the world of gcc, char is considered unsigned by default. It can be forced to be signed with some flag I don't remember at the moment.
We use the default char, so it's unsigned.
In any case, the correct type to hold a "size" is size_t, and not a char, so you're right about the current code being flaky.

[devyte]

You're correct about the last part. In general, it is bad practice to include inside a namespace for this exact reason.
The correct way to handle this is to separate the headers, so a bit more work along the lines of the following. Are you game?

1. ESP8266WebServer-impl.h:
   • Put namespace esp8266webserver around all the code in there.
   • In ESP8266WebServer.h move the include to after the namespace closing like you had previously proposed.
   • Move the three headers added to ESP8266WebServer.h in your latest proposal back to the top of ESP8266WebServer-impl.h.
2. RequestHandler.h: Wrapping with a namespace here could cause havoc in multiple dimensions. To avoid that:
   • Fully qualify the template import like so: using WebServerType = esp8266webserver::ESP8266WebServerTemplate<ServerType>;
3. RequestHandlersImpl.h:
   • Keep your removal of using namespace mime in the global namespace. and its use where specifically used
   • Qualify the using WebServerType same way as above (x2)
4. Move the above includes out of the esp8266 namespace as you had proposed.

[dirkmueller]

@earlephilhower how can I remove the label "merge-conflict" ?