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

Peter peter at pajamian.dhs.org
Wed Dec 31 05:16:42 UTC 2014


On 12/31/2014 05:50 PM, Mark Johnson wrote:
> $Has_JSON is not undeclared. It is defined in the use vars declaration.

Oops, yeah you're right.

> 6 of one, 1/2 dozen of the other.

Yeah, I just figured I'd mention it while mentioning the other stuff.

> 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.

> 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.

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.


Peter



More information about the interchange-users mailing list