On 01 Sep 2018, at 13:06, Yann Ylavic <[email protected]> wrote:
>> 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.
No worries, it caught me just as I was done on apr_jose. Turns out there are a
whole of of security requirements in the RFCs that depend on JSON, requiring a
whole lot of strictness in base64/base64url processing, JSON processing, etc.
What this shows is I need to document apr_json.h better.
>> 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’ve at least twice tried to make the same changes you have, and both times
stopped myself going “oh, yeah”. We definitely need a “simple” signature for
the common case as you describe.
How about this?
/**
* Associate a value with a key in a JSON object.
* @param obj The JSON object.
* @param key Pointer to the key string.
* @param klen Length of the key, or APR_JSON_VALUE_STRING if NUL
* terminated.
* @param val Value to associate with the key.
* @param pool Pool to use.
* @return APR_SUCCESS on success, APR_EINVAL if the key is
* NULL or not a string, or the object is not an APR_JSON_OBJECT.
* @remark If the value is NULL the key value pair is deleted.
*/
APU_DECLARE(apr_status_t) apr_json_object_set(apr_json_value_t *obj,
const char *key, apr_ssize_t klen, apr_json_value_t *val,
apr_pool_t *pool) __attribute__((nonnull(1, 2, 5)));
/**
* Associate a value with a key in a JSON object, preserving whitespace.
* @param obj The JSON object.
* @param key Pointer to the key string, including any whitespace
* required.
* @param val Value to associate with the key.
* @param pool Pool to use.
* @return APR_SUCCESS on success, APR_EINVAL if the key is
* NULL or not a string, or the object is not an APR_JSON_OBJECT.
* @remark If the value is NULL the key value pair is deleted.
*/
APU_DECLARE(apr_status_t) apr_json_object_set_ex(apr_json_value_t *obj,
apr_json_value_t *key, apr_json_value_t *val,
apr_pool_t *pool) __attribute__((nonnull(1, 2, 4)));
Regards,
Graham
—
