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 <san...@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 <ansuels...@gmail.com>
> Cc: Felix Fietkau <n...@openwrt.org>
> Cc: Jan Hoffmann <j...@3e8.eu>
> Cc: Michael Weinrich <mich...@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:
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
