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.

Reply via email to