On 13/11/17 14:44, François Kooman wrote: > On 11/13/2017 01:16 PM, David Sommerseth wrote: [...snip...] >> 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. > > Another very good point. I do have some experience with json-c [1], > would it be acceptable to add this as a project dependency? That is the > main reason I started out by just outputting a string and conveniently > forgot escaping issues...
Yes, I think that should be fine. I see it is available in RHEL6 (the
oldest Linux distribution we support, json-c-0.11-13.el6), so this
should work out just fine. So keep in mind to not use a bleeding fresh
version only available in the latest faster-moving Linux distros.
What would be needed though is to enable/disable this via ./configure
(tackled in configure.ac). We basically can have three approaches here:
a) If json-c is found, enable --status 4 by default. If not found,
this feature is unavailable.
b) Require json-c to be installed, which enables --status 4. To build
without json-c dependency, --disable-json must be used with
./configure
c) Do not require json-c by default, but if adding --enable-json
to ./configure then json-c must be present.
I don't have any strong opinion either way, but I believe alternative c)
is the one which will give the least fuzz in the community - thus
overall be more acceptable.
> So for now my plan is:
>
> 1. use json-c to generate (human readable) JSON;
> 2. also support --status (command line) in addition to "status" in
> management interface
> 3. do not print "END" marker any longer
Sounds like a good plan!
> This is an excellent way for me to gain some more experience with
> writing C (and OpenVPN), even if in the end the patch is not accepted. I
> do think, like Steffan said, we need a good reason to support JSON,
> especially if it adds an additional external dependency. The
> future-proof-ness of the JSON format could be a good selling point,
> maybe new fields would only be added to the JSON format and not to the
> other status formats to promote adoption of the JSON format and slowly
> phasing out the CSV format(s).
That's a good attitude :) I for one is not hard to get sold on the JSON
format though, even though I don't have any particular use right now.
But I can easily see this being preferred for others outside the core
project. And this can simplify some ideas I do have pondered on in my
own infrastructure.
Again the alternative c) approach will fit nicely into this too. I am
definitely willing to enable a JSON feature in the Fedora builds when
patches hits an upstream release. And if that works out nice, I'll
consider to add that even in the EPEL builds too - I'll most likely
mention that on the Fedora devel mailing list to get some thoughts on
the EPEL side before enabling it.
--
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
