[ic] [interchange] Add better messages, support for application/json handling

[David Christensen]

>> It isn't reported as an error in the situation that the module just
>> isn't present. It's advisory, like quite a few other notices that are
>> presented at startup. However, we did debate whether to enable that
>> message or not. Our thoughts were that it would help to advertise the
>> feature once available and, if it were too much to bear, one could start
>> with --quiet.
> 
> Still goes in the global error.log and I'm not aware of any other IC
> code that complains to the log if we do not have an optional module.
> IMO we should (rightly) complain in the case where that module is
> attempted to be used but not otherwise.

I'm okay with taking that part out and just retaining the message if a POST of type application/json is attempted.

>> UnpackJSON is not necessary for handling posts of type application/json.
>> One could just as easily grab the JSON structure out of $CGI::json_ref
>> in global code and go to town. So that alone as an indicator isn't
>> sufficient to determine whether its use is desired.
>> 
>> I have no strong opinion on whether UnpackJSON should be on or off by
>> default, but neither approach is status quo since the entire handler is new.
> 
> I skimmed the code and didn't realize part of it was enabled regardless.
> I was under the impression that UnpackJSON controlled the entire added
> new feature.  That said, having UnpackJSON on vs off is the difference
> of the new feature writing to the CGI space which could in theory
> overwrite CGI vars picked up from the URL or even "mv_" variables that
> are added by interchange, so while it's not very likely it could cause
> some obscure edge cases to have issues if it's on by default.

This is no different than multipart/form-data POST with TolerateGet; it's literally the same codepath, we're just using a different format for the entity body, which is switched based on the request's content-type, so I don't see how this is any different; in fact, IMO this improves the usage of json as a request type (as is done in Angular and some other Javascript frameworks' default handling of AJAX requests).

> Also I would like to disable the entire code block by default as it adds
> additional overhead to processing the page if someone has already
> implemented their own code to handle json data, it may not cause any
> apparent regressions, but there is some overhead in parsing the JSON if
> it's not going to be used.

Short of a custom core patch, how would this have been handled at this level in code?  That kind of situation is always something we cannot anticipate or account for, and the impetus is on them to ensure custom patches still work with upgraded versions of the core.  Considering this is only invoked if POSTed with application/json, I'm not seeing the argument for additional overhead.  Can you elaborate on the issues you see here?
 
Regards,

David
--
David Christensen
End Point Corporation
david at endpoint.com
785-727-1171