Re: [PATCH v3 1/2] json: Add function to unescape JSON-encoded strings

[Patrick Steinhardt]

On Sun, Jun 05, 2022 at 02:00:44PM -0500, Glenn Washburn wrote:
> On Mon, 30 May 2022 18:01:01 +0200
> Patrick Steinhardt <ps@pks.im> 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 <ps@pks.im>
> > ---
> >  grub-core/lib/json/json.c | 98 +++++++++++++++++++++++++++++++++++++++
> >  grub-core/lib/json/json.h | 12 +++++
> >  2 files changed, 110 insertions(+)
> > 
> > diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> > index 1c20c75ea..3e5e25f79 100644
> > --- a/grub-core/lib/json/json.c
> > +++ b/grub-core/lib/json/json.c
> > @@ -262,3 +262,101 @@ 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, size_t *outlen, const char *in, size_t 
> > inlen)
> 
> Why not grub_size_t instead of size_t?
> 
> > +{
> > +  grub_err_t ret = GRUB_ERR_NONE;
> > +  size_t inpos, resultpos;
> 
> Same here.
> 
> > +  char *result;
> > +
> > +  result = grub_calloc (1, inlen + 1);
> 
> Should check for grub_calloc failure.
> 
> I like the idea of being able to do an inplace conversion if in ==
> *out, which would save on memory and be safe as the out string length
> should always be less than or equal to the length of the in string.
> I'll admit its not really necessary for the current use-case.
[snip]

Yeah, I thought about this, and in fact my first iteration did exactly
that. But it complicates the interface somewhat given that you now have
to pass in the caller-allocated size, return the end result's size and
also have to somehow handle NUL-terminating the result. So altogether it
added complexity and felt a lot more fragile compared to just allocating
the result array.

Patrick

signature.asc
Description: PGP signature