* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Sep 24, 2014 at 5:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > ISTM that the most appropriate solution here is to insist that all or none
> > of the members of an operator class be marked leakproof. (Possibly we
> > could restrict that to btree opclasses, but I'm not sure any exception is
> > needed for other index types.) I looked for existing violations of this
> > precept, and unfortunately found a *lot*. For example, texteq is marked
> > leakproof but its fellow text comparison operators aren't. Is that really
> > sane?
>
> Not really. Fortunately, AFAICT, most if not all of these are in the
> good direction: there are some things not marked leakproof that can be
> so marked. The reverse direction would be a hard-to-fix security
> hole. I think at some point somebody went through and tried to mark
> all of the same-type equality operators as leakproof, and it seems
> like that got expanded somewhat without fully rationalizing what we
> had in pg_proc... mostly because I think nobody had a clear idea how
> to do that.
We'll need to investigate and see if there are any which *aren't* safe
and probably fix those to be safe rather than trying to deal with this
risk in some other way. In other words, I hope it's really "all of
these" rather than just most. In general, I've been hoping that we have
more leakproof functions than not as, while it's non-trivial to write
them and ensure they're correct, it's better for us to take that on than
to ask our users to do so (or have them get some set that someone else
wrote off of a website or even the extension network). We can't cover
everything, of course, but hopefully we'll cover all reasonable use
cases for types we ship.
> I think your proposal here is a good one. Heikki proposed treating
> opclass functions as leakproof *in lieu of* adding a flag, but that
> didn't seem good because we wanted to allow for the possibility of
> cases where that wasn't true, and ensure individual scrutiny of each
> case. Your idea of making sure the flag is set consistently
> throughout the opclass (opfamily?) is similar in spirit, but better in
> detail.
Agreed- a regression test here is definitely needed and have any
exceptions which must exist, after we've determined that they don't
produce an actual security hole (not sure how they wouldn't, but still)
explicitly called out in the regression tests, to avoid individuals
missing this requirement by copying an existing pg_proc example and
thinking they can add a new opclass item (or change an existing one) to
not be leakproof when the rest is.
Thanks,
Stephen
