On Mon, Jan 12, 2015 at 05:16:40PM -0500, Stephen Frost wrote: > Alright, here's an updated patch which doesn't return any detail if no > values are visible or if only a partial key is visible.
I browsed this patch. There's been no mention of foreign key constraints, but ri_ReportViolation() deserves similar hardening. If a user has only DELETE privilege on a PK table, FK violation messages should not leak the PK values. Modifications to the foreign side are less concerning, since the user will often know the attempted value; still, I would lock down both sides. Please add a comment explaining the safety of each row-emitting message you haven't changed. For example, the one in refresh_by_match_merge() is safe because getting there requires MV ownership. > --- a/src/backend/access/nbtree/nbtinsert.c > +++ b/src/backend/access/nbtree/nbtinsert.c > @@ -388,18 +388,30 @@ _bt_check_unique(Relation rel, IndexTuple itup, > Relation heapRel, > { > Datum > values[INDEX_MAX_KEYS]; > bool > isnull[INDEX_MAX_KEYS]; > + char *key_desc; > > index_deform_tuple(itup, > RelationGetDescr(rel), > > values, isnull); > - ereport(ERROR, > - > (errcode(ERRCODE_UNIQUE_VIOLATION), > - > errmsg("duplicate key value violates unique constraint \"%s\"", > - > RelationGetRelationName(rel)), > - errdetail("Key > %s already exists.", > - > BuildIndexValueDescription(rel, > - > values, isnull)), > - > errtableconstraint(heapRel, > - > RelationGetRelationName(rel)))); > + > + key_desc = > BuildIndexValueDescription(rel, values, > + > isnull); > + > + if (!strlen(key_desc)) > + ereport(ERROR, > + > (errcode(ERRCODE_UNIQUE_VIOLATION), > + > errmsg("duplicate key value violates unique constraint \"%s\"", > + > RelationGetRelationName(rel)), > + > errtableconstraint(heapRel, > + > RelationGetRelationName(rel)))); > + else > + ereport(ERROR, > + > (errcode(ERRCODE_UNIQUE_VIOLATION), > + > errmsg("duplicate key value violates unique constraint \"%s\"", > + > RelationGetRelationName(rel)), > + > errdetail("Key %s already exists.", > + > key_desc), > + > errtableconstraint(heapRel, > + > RelationGetRelationName(rel)))); Instead of duplicating an entire ereport() to change whether you include an errdetail, use "condition ? errdetail(...) : 0". -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers