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


Attachment: 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

Reply via email to