Hi,
On Fri, Aug 14, 2026 at 6:08 PM Masahiko Sawada <[email protected]> wrote:
>
> > > >> +static void
> > > >> +string_to_uuid(const char *source, pg_uuid_t *uuid, Node *escontext)
> > > >> +{
> > > >> + const char *body = source;
> > > >> + size_t len = strlen(source);
> > > >> ```
> > > >>
> > > >> I think it would be better to avoid strlen(). The old code processes
> > > >> at most UUID_LEN (16) byte pairs, so it does not need to scan
> > > >> arbitrarily far on malformed input. So, maybe we could use something
> > > >> like strnlen(source, 39) instead.
> > > >
> > > > While strnlen(source, 39) works there, 39 is a magic number and it's
> > > > tied to the current format check logic. What is the benefit of using
> > > > strnlen(source, 39) instead? I'm not sure it warrants having the magic
> > > > number.
> > >
> > > It doesn't have to be exactly 39; 1024 (long enough) would also work, or
> > > perhaps something based on UUID_LEN, such as UUID_LEN * 3. I think the
> > > main point is to avoid unbounded scanning on malformed input.
> > >
> > > The old code did not have this issue because it only examined as much
> > > input as needed based on UUID_LEN. The new fast path starts to use
> > > strlen(), so this would be a new risk introduced by the optimization.
> >
> > I don't think the scan can be really unbounded. string_to_uuid()
> > receives a cstring, so by the time it is called the caller has already
> > walked or copied the whole string to produce it. So unless the
> > unbounded scan can be reached in some path I have overlooked, I'd
> > prefer to keep strlen() here. Happy to change it if you still think it
> > is worth it.
>
> After more thoughts, while I still don't think the scan can be
> unbounded, using strlen() would add an extra scan just to determine we
> use hex_decode_safe(). I'll change it to use strnlen() instead.
+1 to use strnlen() instead of strlen() just for the simple reason
that it's better not to scan malformed-yet-null-terminated input
strings (say a null-terminated input of 1GB).
DO $$ DECLARE s text := repeat('a', 1000000000); r uuid;
BEGIN r := s::uuid; EXCEPTION WHEN others THEN NULL; END $$;
(gdb) p strlen(uuid_str)
$2 = 1000000000
Instead of N * UUID_LEN in strnlen, NAMEDATALEN also works unless I'm
missing anything.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
