On Mon, Jun 06, 2022 at 07:28:56AM +0200, Patrick Steinhardt wrote:
> JSON strings require certain characters to be encoded, either by using a
> single reverse solidus character "\" for a set of popular characters, or
> by using a Unicode representation of "\uXXXXX". The jsmn library doesn't
> handle unescaping for us, so we must implement this functionality for
> ourselves.
>
> Add a new function `grub_json_unescape ()` that takes a potentially
> escaped JSON string as input and returns a new unescaped string.
>
> Signed-off-by: Patrick Steinhardt <p...@pks.im>
> ---
> grub-core/lib/json/json.c | 101 ++++++++++++++++++++++++++++++++++++++
> grub-core/lib/json/json.h | 12 +++++
> 2 files changed, 113 insertions(+)
>
> diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> index 1c20c75ea..adb4747a4 100644
> --- a/grub-core/lib/json/json.c
> +++ b/grub-core/lib/json/json.c
> @@ -262,3 +262,104 @@ grub_json_getint64 (grub_int64_t *out, const
> grub_json_t *parent, const char *ke
>
> return GRUB_ERR_NONE;
> }
> +
> +grub_err_t
> +grub_json_unescape (char **out, grub_size_t *outlen, const char *in,
> grub_size_t inlen)
> +{
> + grub_err_t ret = GRUB_ERR_NONE;
> + grub_size_t inpos, resultpos;
> + char *result;
> +
> + if (!out || !outlen)
I prefer "if (out == NULL || outlen == NULL)"
Please fix similar issues below.
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Output parameters are not
> set");
grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are not set"))
Two things are important here. First, you should use N_() in
grub_error() calls. It enables i18n for the GRUB. Second, all
grub_error() messages should start with lowercase except e.g.
abbreviations.
Please fix similar things below.
> +
> + result = grub_calloc (1, inlen + 1);
> + if (!result)
> + return GRUB_ERR_OUT_OF_MEMORY;
> +
> + for (inpos = resultpos = 0; inpos < inlen; inpos++)
> + {
> + if (in[inpos] == '\\')
> + {
> + inpos++;
> + if (inpos >= inlen)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Expected escaped
> character");
> + goto err;
> + }
> +
> + switch (in[inpos])
> + {
> + case '"':
> + result[resultpos++] = '"'; break;
Please split this line into two...
result[resultpos++] = '"';
break;
> + case '/':
> + result[resultpos++] = '/'; break;
result[resultpos++] = '/';
break;
> + case '\\':
> + result[resultpos++] = '\\'; break;
Ditto and below please...
> + case 'b':
> + result[resultpos++] = '\b'; break;
> + case 'f':
> + result[resultpos++] = '\f'; break;
> + case 'r':
> + result[resultpos++] = '\r'; break;
> + case 'n':
> + result[resultpos++] = '\n'; break;
> + case 't':
> + result[resultpos++] = '\t'; break;
> + case 'u':
> + {
> + unsigned char values[4] = {0};
Why values is unsigned char if c is char. I think values should be char too.
> + int i;
> +
> + inpos++;
> + if (inpos + ARRAY_SIZE(values) > inlen)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unicode
> sequence too short");
> + goto err;
> + }
> +
> + for (i = 0; i < 4; i++)
"i < ARRAY_SIZE(values)" instead of "i < 4"?
> + {
> + char c = in[inpos++];
> +
> + if (c >= '0' && c <= '9')
> + values[i] = c - '0';
> + else if (c >= 'A' && c <= 'F')
> + values[i] = c - 'A' + 10;
> + else if (c >= 'a' && c <= 'f')
> + values[i] = c - 'a' + 10;
> + else
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
> + "Unicode sequence with invalid
> character '%c'", c);
> + goto err;
> + }
> + }
> +
> + if (values[0] != 0 || values[1] != 0)
> + result[resultpos++] = values[0] << 4 | values[1];
> + result[resultpos++] = values[2] << 4 | values[3];
> +
> + /* Offset the increment that's coming in via the loop
> increment. */
> + inpos--;
> +
> + break;
> + }
Please add empty line here and above after every break. OK, the first
one above does not need an additional empty line...
> + default:
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unrecognized escaped
> character '%c'", in[inpos]);
> + goto err;
> + }
> + }
> + else
> + result[resultpos++] = in[inpos];
> + }
> +
> + *out = result;
> + *outlen = resultpos;
> +
> +err:
Please add a space before "err" label.
> + if (ret != GRUB_ERR_NONE)
> + grub_free (result);
> +
> + return ret;
> +}
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
