Add header access using same method as arguments

[bren3582]

Here's my fix for issue #795 which adds access to request headers by the same method that arguments are accessed. I needed to access the Cookie header and now I can do that like this.

if (server.hasHeader("Cookie")) {
    String cookieValue = server.header("Cookie");
}

[drewfish]

I worry about the memory and CPU overhead, specifically if I never use this new API. Ideally hasHeader() and header() would only create that internal object if/when these functions are called, that way those folks who don't use them don't have to pay the cost.

[bren3582]

@drewfish, that's definitely a valid concern and I think I have a good solution to it. I'll update the code later today once I get everything working.

[bren3582]

I've updated the method of collecting request headers so that it is off by default in response to @drewfish's resource concerns. Now if I want to have access to the User-Agent and Cookie headers for example...

const size_t headerKeysCount = 2;
const char* headerKeys[headerKeysCount] = {"User-Agent", "Cookie"};
server.collectHeaders(headerKeys, headerKeysCount);

The methods to access the collected headers remain the same.

[drewfish]

Do we also maybe want to do _headerKeysCount = 0 just to be safe?

[bren3582]

I doubt it because the object is being destroyed anyway but why not, can't be too safe. Does the rest of the PR work for you?

[drewfish]

Will this end up copying the returned String (or will the compiler optimize)? Is there a "best practice" (examples in other well-used libraries) on how to return a String efficiently?

(Am I focusing too much on optimizations 😄 😞 ?)

[drewfish]

For example, would const String& be a better return type? (Will be tricky to handle the "not found" case, but we could make a single empty string to use for all "not found" return values.)

[bren3582]

If you look just above (ESP8266WebServer.h Line 81), you'll see that the existing argument access method is the same. There may be a better way but I decided to follow the conventions of the existing code.

[drewfish]

OK, cool, good enough for me.

[drewfish]

I like the approach.

I'm uncertain about fine-tuning the performance, since I'm not that experienced with microprocessor (and specifically esp8266) programming, so I'm fine with whatever practice is common in this world.

[drewfish]

+1 from me

[igrr]

Looks mostly good to me. Few comments from my side:

• adding a usage example would be great
• who is supposed to own _headerKeys? The calling code or the ESP8266WebServer?
• let's use new/delete[] instead of malloc/free in new code

[luc-github]

Hi looks very nice and I am really willing to use it ( I need a cookie 😄 )
any time frame for modification/merging ?

[luc-github]

I would be happy to help doing modification - I just do not want overlap

also it won't be better instead of free / allocate the header array at each page, to allocate one for all ?
As number of keys is fixed at the beginning.

So we could directly copy the requested keys in _currentHeaders[i].key at the initialization and just set _currentHeaders[i].value="" for each reset, instead of delete all array and allocate it again.
because _collectHeader(headerName.c_str()) already look if it requested - so instead of boolean answer just give the position in _currentHeaders[] to put the value or -1 if not requestedjust assign value when found

so no need to allocate the Keys array and no need to delete/allocate the header array at each page

[luc-github]

@brianensor @igrr I have done modifications I mentioned and which match @igrr request, and did a sample using header for simple authentication using cookie (login page / disconnection/etc...) - if no comment against, I will do a PR tonight, I hope it will be fine, if not I will adjust it

[igrr]

@luc-github please do, this should be merged before Friday.

[luc-github]

Ok just did #933
If anything need to be changed, I will do