On Sat, Sep 1, 2018 at 11:53 AM Graham Leggett <minf...@sharp.fm> wrote: > > On 31 Aug 2018, at 19:56, yla...@apache.org wrote: > > Author: ylavic > Date: Fri Aug 31 17:56:40 2018 > New Revision: 1839779 > > URL: http://svn.apache.org/viewvc?rev=1839779&view=rev > Log: > apr_json: follow up to r1839755: preserve formatting when decoding object > valueT > > The object key-value pair parsed by the JSON decoder contained pre/post spaces > which we lost by changing the key type (to a C string) in > apr_json_object_set(). > > Add a new/private apr__json_object_set() helper taking an apr_json_value_t key > and use it in both apr_json_object_set() and apr_json_decode_object(). > > > Looking at 1839779 and 1839755, both of these need to be reverted as they > break RFC compliance with respect to the JSON RFC and JOSE RFCs (I’m -1 on > these changes).
There is probably another solution, see below. > > Please can you discuss these changes first before making them, I’ve had my > head buried in these RFCs for the past while and I can explain the design > choices behind the API, and add any documentation that’s lacking to clarify > how this works. OK, sorry about this. > > JSON whitespace is significant in some RFCs, specifically the JWE and JWS > RFCs, and there needs to be a mechanism to formally support whitespace for > this API to be be useful in a security sensitive environment. My changes were meant to allow simple (default) apr_json_object_set without having to create a json_string for the key. That's IMO how most users want it to be: apr_json_object_set(json, "somekey", apr_json_<type>_create(...)); rather than: apr_json_object_set(json, apr_json_string_create("somekey"), apr_json_<type>_create(...)); ... > > I am currently blocked behind these changes, so I’m going to revert them for > now so that I can get the JOSE code in. We can discuss this in more detail > once that is done. I suppose what you want from my changes is to make the private apr__json_object_set available in the API and use it for apr_jose (e.g. with a different name like apr_json_object_put or something like that). I find apr_json_object_set with a plain C string key more friendly in the usual case... Regards, Yann.