[PATCH libubox] jshn: Do not perform double key normalisation

[Sander Vanheule]

Cc Felix's correct address. Sorry for the noise!

Best,
Sander

On Sun, 2023-10-22 at 17:51 +0200, Sander Vanheule wrote:
> jshn stores a JSON file in the shell environment using a set of
> variable, using the key to an object in the shell variable name. This
> restricts the allowed character set to numbers, letters, and underscore.
> 
> Commit a1a97eb11e89 ("jshn: support using characters in elements that do
> not conform to shell variable restrictions") added normalisation to
> jshn.sh, to ensure the shell functions produce legal variable names. The
> original key name is stored in a 'N_normalised_key' variable, to enable
> 'jshn -o file.json' to create a JSON file with the original key name.
> 
> Earlier code from commit fda6079b30a4 ("add jshn") also included the
> necessary code to perform character set normalisation while building
> variable names, which is triggered when loading files with 'jshn -R
> file.json'. While add_json_element() will normalise key names, it will
> not produce the key names lookup variables, as this concept had not been
> introduced yet.
> 
> The former change resulted in double name normalisation being performed
> when loading an existing file into the environment, causing the original
> key name to be replaced by its normalised version:
> 
>         # cat board.json 
>         {
>                 "network-device": {
>                         "foo": "bar"
>                 }
>         }
> 
>         # /usr/bin/jshn -R board.json 
>         json_init;
>         json_add_object 'network_device';
>         json_add_string 'foo' 'bar';
>         json_close_object;
> 
> By removing key normalisation from jshn.c, _json_add_generic() now has
> the required information again to correctly store the original key,
> ensuring repeated operations of 'jshn -R' and 'jshn -o' do not modify
> the original JSON file (modulo formatting).
> 
> Fixes: a1a97eb11e89 ("jshn: support using characters in elements that do not conform to
> shell variable restrictions")
> Signed-off-by: Sander Vanheule <sander at svanheule.net>
> ---
> This issue was uncovered by the people debugging a recent issue on
> Realtek switches [1]. A JSON section was used with the name
> 'network-device'. As per the above description, this would be normalised
> to 'network_device'. The key name was correctly stored, as long as the
> file adding that section (02_network) was the last one to be run. 
> 
> OpenWrt commit 4ebba8a05d09 ("realtek: add support for HPE
> 1920-8g-poe+") had added a file 03_gpio running after 02_network,
> resulting a call to 'jshn -R' and silent normalisation of
> 'network-device' to 'network_device'.
> 
> The issue was fixed in OpenWrt and netifd by just always using the
> normalised key version. Note that even with this change applied, there
> is still no way with jshn to represent keys with the same normalisation
> (e.g. 'foo-bar' and 'foo_bar') in one file. Fixing that would require
> properly escaping key names in some way when building variable names.
> 
> [1] https://forum.openwrt.org/t/57875/2589
> 
> Cc: Christian Marangi <ansuelsmth at gmail.com>
> Cc: Felix Fietkau <nbd at openwrt.org>
> Cc: Jan Hoffmann <jan at 3e8.eu>
> Cc: Michael Weinrich <michael at a5ap.net>
> 
> 
>  jshn.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/jshn.c b/jshn.c
> index 1b685e5fb0d8..97873a220d64 100644
> --- a/jshn.c
> +++ b/jshn.c
> @@ -96,14 +96,6 @@ static void add_json_string(const char *str)
>                 fwrite(ptr, len, 1, stdout);
>  }
>  
> -static void write_key_string(const char *key)
> -{
> -       while (*key) {
> -               putc(isalnum(*key) ? *key : '_', stdout);
> -               key++;
> -       }
> -}
> -
>  static int add_json_element(const char *key, json_object *obj)
>  {
>         char *type;
> @@ -135,7 +127,7 @@ static int add_json_element(const char *key, json_object *obj)
>         }
>  
>         fprintf(stdout, "json_add_%s '", type);
> -       write_key_string(key);
> +       add_json_string(key);
>  
>         switch (json_object_get_type(obj)) {
>         case json_type_object: