Andres Freund <and...@anarazel.de> writes:
> On 2019-07-30 11:54:55 -0700, Jeff Davis wrote:
>> My proposal is:
>> * redact every '%s' in an ereport by having a special mode for
>> snprintf.c (this is possible because we now own snprintf)

> I'm extremely doubtful this is a sane approach.

Yeah, it's really hard to believe that messing with snprintf isn't
going to have a lot of unintended consequences.  If we want to do
something about this, we're going to have to bite the bullet and
go around to annotate all those arguments with redaction-worthiness
info.  It'd be a lot of work, but as long as we can make sure to
do it incrementally, we could get there.  (It's not like ereport
itself has been there forever ...)

> I.e. something very roughly like

> ereport(ERROR,
>         errmsg_rich("string with %{named}s references to %{parameter}s"),
>         errparam("named", somevar),
>         errparam("parameter", othervar, .redact = CONTEXT));

I'm not terribly attracted by that specific approach, though.  With
this, we'd get to maintain *two* variants of snprintf, and I think
the one with annotation knowledge would have significant performance
problems.  (Something I'm sensitive to, on account of certain persons
beating me over the head about snprintf.c's performance.)  It'd be
an amazing pain in the rear for translators, too, on account of
their tools not understanding this format-string language.

It seems to me that it'd be sufficient to do the annotation by
inserting wrapper functions, like the errparam() you suggest above.
If we just had errparam() choosing whether to return "..." instead of
its argument string, we'd have what we need, without messing with
the format language.

                        regards, tom lane


Reply via email to