On Mon, Jul 11, 2022 at 09:08:09AM -0400, Nicholas Vinson wrote:
> On 7/11/22 06:44, 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 | 118 ++++++++++++++++++++++++++++++++++++++
> > grub-core/lib/json/json.h | 12 ++++
> > 2 files changed, 130 insertions(+)
> >
> > diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> > index 1c20c75ea..1eadd1ce9 100644
> > --- a/grub-core/lib/json/json.c
> > +++ b/grub-core/lib/json/json.c
> > @@ -262,3 +262,121 @@ 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 == NULL || outlen == NULL)
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are
> > not set"));
> > +
> > + result = grub_calloc (1, inlen + 1);
> > + if (result == NULL)
> > + 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, N_("expected escaped
> > character"));
> > + goto err;
> > + }
> > +
> > + switch (in[inpos])
> > + {
> > + case '"':
> > + result[resultpos++] = '"';
> > + break;
> > +
> > + case '/':
> > + result[resultpos++] = '/';
> > + break;
> > +
> > + case '\\':
> > + result[resultpos++] = '\\';
> > + break;
> > +
> > + 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':
> > + {
> > + char values[4] = {0};
> > + unsigned i;
> > +
> > + inpos++;
> > + if (inpos + ARRAY_SIZE(values) > inlen)
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode
> > sequence too short"));
> > + goto err;
> > + }
>
> I recommend relaxing these errors a little bit. While technically
> correct, I'm thinking it might serve GRUB better to be a bit more
> forgiving.
>
> Perhaps something like:
>
> if the input is '\u????' where ? is not a hex digit, set the value to
> 'u', and process ???? separately.
>
> if ???? is less than ARRAY_SIZE chars, left-pad ???? with 0s* until it's
> the correct length then convert the value.
>
> (* I don't mean to literally pad. I mean to treat the short number as if
> it was left-padded with 0s)
>
> > +
> > + for (i = 0; i < ARRAY_SIZE(values); i++)
> > + {
> > + 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
> > + {
>
> With a similar argument here. Treat the short ???? as suggested above
> and resume normal processing at the non-hex digit position.
>
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
> > + N_("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;
> > + }
> > +
> > + default:
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized
> > escaped character '%c'"), in[inpos]);
> > + goto err;
>
> And the final prong of the suggestion would be here. Treat the improper
> escape as if it had not been escaped at all or as '\\?' where '?' is the
> escaped character.
Nicholas comments make sense for me.
Patrick?
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
