On 2024-03-12 Tu 14:43, Jacob Champion wrote:
Hello,
Both the incremental JSON [1] and OAuth [2] patchsets would be
improved by json_errdetail(), which was removed from FRONTEND builds
in b44669b2ca:
The routine providing a detailed error message based on the error code
is made backend-only, the existing code being unsafe to use in the
frontend as the error message may finish by being palloc'd or point to a
static string, so there is no way to know if the memory of the message
should be pfree'd or not.
Attached is a patch to undo this, by attaching any necessary
allocations to the JsonLexContext so the caller doesn't have to keep
track of them.
This is based on the first patch of the OAuth patchset, which
additionally needs json_errdetail() to be safe to use from libpq
itself. Alvaro pointed out offlist that we don't have to go that far
to re-enable this function for the utilities, so this patch is a sort
of middle ground between what we have now and what OAuth implements.
(There is some additional minimization that could be done to this
patch, but I'm hoping to keep the code structure consistent between
the two, if the result is acceptable.)
Seems reasonable.
Two notes that I wanted to point out explicitly:
- On the other thread, Daniel contributed a destroyStringInfo()
counterpart for makeStringInfo(), which is optional but seemed useful
to include here.
yeah, although maybe worth a different patch.
- After this patch, if a frontend client calls json_errdetail()
without calling freeJsonLexContext(), it will leak memory.
Not too concerned about this:
1. we tend to be a bit more relaxed about that in frontend programs,
especially those not expected to run for long times and especially on
error paths, where in many cases the program will just exit anyway.
2. the fix is simple where it's needed.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
