Bug: Processing for header field HTTP_X_REQUESTED_WITH is not enabled.

[yunjova]

There is a bug in the file: "Sming/Sming/SmingCore/Network/HttpServer.cpp":
Please add the statement: enableHeaderProcessing("HTTP_X_REQUESTED_WITH");
This is needed to make the function bool HttpRequest::isAjax() in Sming/Sming/SmingCore/Network/HttpRequest.cpp work as is expected.

[yunjova]

Additional info:

To be added in function: HttpServer::HttpServer() of file: "Sming/Sming/SmingCore/Network/HttpServer.cpp".

[patrickjahns]

For now you can enable it yourself when initializing your server

[yunjova]

OK, but please add the statement when a new version is released.

[patrickjahns]

@hreintke

I wonder if it is necessary to keep it enabled as default, or if we document that in order to use this, the user of Sming should enable it.

The reason for asking is, that defaultwise we do not need this header and for most applications it is just wasted memory to always process it

[yunjova]

You could document it in the isAjax() function. (This is the minimal to avoid that people have to search why it "does not work".)
Better would be (on top of the documentation) that the isAjax() function reports an error/log when it is called and the header is not marked as to-be-processed.

[hreintke]

@patrickjahns @yunjova Sorry for the late response.
I agree with @yunjova that if we use this header explicit in the isajax() function, we should enable it by default.
@patrickjahns : Do you mean wasted memory when the header is added to the processingheaders vector ?
Looking to the functionality of enableheaderprocessing("xxxx") which has been in sming from the very first. Shouldn't e always put all headers in the processingheaders vector. Why should we limit this.

[patrickjahns]

@hreintke

I`d prefer the current implementation over enabling every header by default. Sometimes the Webbrowser / Clients send a lot of not needed header information which would be kept in memory with every request. So depending on the request - the amount of memory available to actually processing the required headers/body decreases unnecessarily.

If someone is interested in a particular header in his application he can enable processing.

isAjax()
To be honest - I don't see this implementation as very useful - it would make more sense if we would also provide isJson() and other functionality to be inline. Otherwise is see this to be more as part of the users application and would only enable the parsing for the header, if this is required.
But that is personal preference - don`t make the ram/cpu cycle usage heavier for everyone- if it is only needed very few times

[hreintke]

@patrickjahns :
Understand your reasoning and agree.
That would mean we remove the isAjax() from Sming and any other similar.
Maybe we should include something like :
enableProcessingHeaders(ALL) and enableProcessingHeaders(CONTAINS) (other ??) to ease the setting of enableProcessingHeaders.

[patrickjahns]

@hreintke
Let`s move these functional aspects into the appropriate samples - this way people would still see how it is done in their application then

I like the idea with ALL - if not ALL I propose to keep it as it currently is and only enable the barebone necessary headers (to be extended by the application with enableProcessingHeaders("myheader")

[hreintke]

@patrickjahns :
I was thinking on CONTAINS to (f.e) enable applications to only get the headers which are custom to it.
X-APP-CUSTOM1
X-APP-CUSTOM2
without this an application would never be able to detect sending of invalid X-APP-XXX headers without always having the ALL option activated, resulting in large memory usage.

[patrickjahns]

@hreintke
I see your idea - why not explicitly set it?

enableHeaderProcessing("X-APP-CUSTOM1");
enableHeaderProcessing("X-APP-CUSTOM2");

My fear is, that while ALL can be easily done by setting an internal boolean var - having a CONTAINS would make parsing more complex. i.e. does contains match STRING or does it match STRING* or *STRING ?

Also not sure how the interface towards the application should be - will we allow processing of several checks (i.e. contains X-APP contains X-AUTH etc.)

[hreintke]

@patrickjahns : Reason to give the application some more flexibility and not requiring a (lot of) detailed hardcoding or high memory usage.

Interface can be identical as we have now :
void HttpServer::enableHeaderProcessing(String headerName)
add void HttpServer::enableHeaderProcessing(String headerName, bool contains)
Saving the contains in the processingheaders vector.

And adding the check within :
bool HttpServer::isHeaderProcessingEnabled(String name)
with if processingheaders.indexof(name) != 0 -> is enabled

[patrickjahns]

Thanks for clarification - I can create a PR with:

• isAjax() moved to examples using it
• more flexibel headerParsing

(since I am quite familiar with the HttpServer part anyhow)

[hreintke]

@patrickjahns :
OK, Thanks for putting this on your list.

@yunjova :
Probably the discussion did not quite go the way you proposed but by implementing in this way we

• remove unclear/not complete Sming functionality.
• improve the way an application can handle this in a generic way.

[slaff]

With the latest Http refactoring (#1112) that issue should not longer be relevant.