On 14/11/17 09:31, Gert Doering wrote: > Hi, > > On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote: >> But we should consider if we want to make use of a JSON library >> producing the JSON streams. The reason is to ensure the output is >> according to the specification and that escaping if contents is >> consistent and proper. Imagine if someone puts a double-quote into the >> CN field of a certificate? >> >> CN="} Lets explode things, O=Hacktivist0 >> >> Or other characters which needs escaping. > > I'm not convinced we want to import a new library dependency and a heap > of #ifdef for this. > > Escaping on *output* is pretty trivial ("characters from <this set> > need to be encoded <like this>") - and as long as we do not need to parse > *incoming* JSON, a full-blown new library is mainly adding complications > (like, configure flags, #ifdefs, library version dependencies, ...). > > But you knew that this response would be coming :-) Not surprised ;-)
But I do disagree. I tend to dislike re-inventing wheels when others do the job for you. Quite some years ago, I would probably have agreed with you. But I've been in projects where I implemented producing XML data manually in a similar manner. After fixing numerous of escaping errors, I switched to a library and all these bugs disappeared and never came back. It is so easy to think you know exactly what needs to be escaped beforehand and firmly believe you have covered each corner-case, but it always sneaks back at you - as you didn't spot the hidden doors. And to me this feels like it actually contradicts our netlink discussion on the Hackathon, where you initially wanted to use an external library ... ;-) That said, json-c is packaged and available on Linux, FreeBSD and even OpenWRT/LEDE got a recent version available. I don't know the situation on macOS (but that's similar to FreeBSD, I'd presume) and Windows is also not clear to me (if this is a relevant feature there). json-c is also fairly small (approx 7k lines of code, stripped binary lib ~60KB) and is being maintained (last commits from late October). So this isn't a big massive dependency. With ./configure --enable-json we also don't change the current behaviour in OpenVPN. Which should only require a single #ifdef ENABLE_JSON in the multi_print_status() [multi.c:859] function. But I also see that the status printing stuff could be improved as well, to simplify the implementation a bit too (which is a different task). -- kind regards, David Sommerseth OpenVPN, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel