Fix some corner cases in ACL manipulation: don't foul up on an empty

ACL array, and force languages to be treated as owned by the bootstrap
user ID.  (pg_language should have a lanowner column, but until it does
this will have to do as a workaround.)

src/backend/catalog/aclchk.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.89 2003/10/05 21:49:12 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.90 2003/10/29 22:20:54 tgl Exp $
  *
  * NOTES
  *	  See acl.h.
@@ -106,7 +106,7 @@ merge_acl_with_grant(Acl *old_acl, bool is_grant,
 
 			idtype = ACL_IDTYPE_UID;
 
-			grantee_is_owner = (aclitem.ai_grantee == owner_uid && owner_uid != InvalidOid);
+			grantee_is_owner = (aclitem.ai_grantee == owner_uid);
 		}
 		else if (grantee->groupname)
 		{
@@ -550,12 +550,15 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 
 		/*
 		 * If there's no ACL, create a default.
+		 *
+		 * Note: for now, languages are treated as owned by the bootstrap
+		 * user.  We should add an owner column to pg_language instead.
 		 */
 		aclDatum = SysCacheGetAttr(LANGNAME, tuple, Anum_pg_language_lanacl,
 								   &isNull);
 		if (isNull)
 			old_acl = acldefault(ACL_OBJECT_LANGUAGE,
-								 InvalidOid);
+								 BOOTSTRAP_USESYSID);
 		else
 			/* get a detoasted copy of the ACL */
 			old_acl = DatumGetAclPCopy(aclDatum);
@@ -563,7 +566,7 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grantees, privileges,
 									   stmt->grant_option, stmt->behavior,
-									   InvalidOid);
+									   BOOTSTRAP_USESYSID);
 
 		/* finished building new ACL value, now insert it */
 		MemSet(values, 0, sizeof(values));
@@ -1205,7 +1208,8 @@ pg_language_aclcheck(Oid lang_oid, AclId userid, AclMode mode)
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		acl = acldefault(ACL_OBJECT_LANGUAGE, InvalidOid);
+		/* XXX pg_language should have an owner column, but doesn't */
+		acl = acldefault(ACL_OBJECT_LANGUAGE, BOOTSTRAP_USESYSID);
 		aclDatum = (Datum) 0;
 	}
 	else

src/backend/utils/adt/acl.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.99 2003/09/25 06:58:03 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.100 2003/10/29 22:20:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -542,26 +542,23 @@ acldefault(GrantObjectType objtype, AclId ownerid)
 			break;
 	}
 
-	acl = allocacl((world_default != ACL_NO_RIGHTS ? 1 : 0)
-				   + (ownerid ? 1 : 0));
+	acl = allocacl((world_default != ACL_NO_RIGHTS) ? 2 : 1);
 	aip = ACL_DAT(acl);
 
 	if (world_default != ACL_NO_RIGHTS)
 	{
-		aip[0].ai_grantee = ACL_ID_WORLD;
-		aip[0].ai_grantor = ownerid;
-		ACLITEM_SET_PRIVS_IDTYPE(aip[0], world_default, ACL_NO_RIGHTS, ACL_IDTYPE_WORLD);
+		aip->ai_grantee = ACL_ID_WORLD;
+		aip->ai_grantor = ownerid;
+		ACLITEM_SET_PRIVS_IDTYPE(*aip, world_default, ACL_NO_RIGHTS,
+								 ACL_IDTYPE_WORLD);
+		aip++;
 	}
 
-	if (ownerid)
-	{
-		int			index = (world_default != ACL_NO_RIGHTS ? 1 : 0);
-
-		aip[index].ai_grantee = ownerid;
-		aip[index].ai_grantor = ownerid;
-		/* owner gets default privileges with grant option */
-		ACLITEM_SET_PRIVS_IDTYPE(aip[index], owner_default, owner_default, ACL_IDTYPE_UID);
-	}
+	aip->ai_grantee = ownerid;
+	aip->ai_grantor = ownerid;
+	/* owner gets default privileges with grant option */
+	ACLITEM_SET_PRIVS_IDTYPE(*aip, owner_default, owner_default,
+							 ACL_IDTYPE_UID);
 
 	return acl;
 }
@@ -574,17 +571,22 @@ acldefault(GrantObjectType objtype, AclId ownerid)
  * NB: caller is responsible for having detoasted the input ACL, if needed.
  */
 Acl *
-aclinsert3(const Acl *old_acl, const AclItem *mod_aip, unsigned modechg, DropBehavior behavior)
+aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
+		   unsigned modechg, DropBehavior behavior)
 {
 	Acl		   *new_acl = NULL;
 	AclItem    *old_aip,
 			   *new_aip = NULL;
+	AclMode		old_privs,
+				old_goptions,
+				new_privs,
+				new_goptions;
 	int			dst,
 				num;
 
 	/* These checks for null input are probably dead code, but... */
-	if (!old_acl || ACL_NUM(old_acl) < 1)
-		old_acl = allocacl(1);
+	if (!old_acl || ACL_NUM(old_acl) < 0)
+		old_acl = allocacl(0);
 	if (!mod_aip)
 	{
 		new_acl = allocacl(ACL_NUM(old_acl));
@@ -629,16 +631,23 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip, unsigned modechg, DropBeh
 		num++;					/* set num to the size of new_acl */
 	}
 
-	/* apply the permissions mod */
+	old_privs = ACLITEM_GET_PRIVS(new_aip[dst]);
+	old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
+
+	/* apply the specified permissions change */
 	switch (modechg)
 	{
 		case ACL_MODECHG_ADD:
-			ACLITEM_SET_PRIVS(new_aip[dst], ACLITEM_GET_PRIVS(new_aip[dst]) | ACLITEM_GET_PRIVS(*mod_aip));
-			ACLITEM_SET_GOPTIONS(new_aip[dst], ACLITEM_GET_GOPTIONS(new_aip[dst]) | ACLITEM_GET_GOPTIONS(*mod_aip));
+			ACLITEM_SET_PRIVS(new_aip[dst],
+							  old_privs | ACLITEM_GET_PRIVS(*mod_aip));
+			ACLITEM_SET_GOPTIONS(new_aip[dst],
+								 old_goptions | ACLITEM_GET_GOPTIONS(*mod_aip));
 			break;
 		case ACL_MODECHG_DEL:
-			ACLITEM_SET_PRIVS(new_aip[dst], ACLITEM_GET_PRIVS(new_aip[dst]) & ~ACLITEM_GET_PRIVS(*mod_aip));
-			ACLITEM_SET_GOPTIONS(new_aip[dst], ACLITEM_GET_GOPTIONS(new_aip[dst]) & ~ACLITEM_GET_GOPTIONS(*mod_aip));
+			ACLITEM_SET_PRIVS(new_aip[dst],
+							  old_privs & ~ACLITEM_GET_PRIVS(*mod_aip));
+			ACLITEM_SET_GOPTIONS(new_aip[dst],
+								 old_goptions & ~ACLITEM_GET_GOPTIONS(*mod_aip));
 			break;
 		case ACL_MODECHG_EQL:
 			ACLITEM_SET_PRIVS_IDTYPE(new_aip[dst],
@@ -648,10 +657,13 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip, unsigned modechg, DropBeh
 			break;
 	}
 
+	new_privs = ACLITEM_GET_PRIVS(new_aip[dst]);
+	new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
+
 	/*
 	 * If the adjusted entry has no permissions, delete it from the list.
 	 */
-	if (ACLITEM_GET_PRIVS(new_aip[dst]) == ACL_NO_RIGHTS)
+	if (new_privs == ACL_NO_RIGHTS && new_goptions == ACL_NO_RIGHTS)
 	{
 		memmove(new_aip + dst,
 				new_aip + dst + 1,
@@ -661,12 +673,14 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip, unsigned modechg, DropBeh
 	}
 
 	/*
-	 * Remove abandoned privileges (cascading revoke)
+	 * Remove abandoned privileges (cascading revoke).  Currently we
+	 * can only handle this when the grantee is a user.
 	 */
-	if (modechg != ACL_MODECHG_ADD
-		&& ACLITEM_GET_IDTYPE(*mod_aip) == ACL_IDTYPE_UID
-		&& ACLITEM_GET_GOPTIONS(*mod_aip))
-		new_acl = recursive_revoke(new_acl, mod_aip->ai_grantee, ACLITEM_GET_GOPTIONS(*mod_aip), behavior);
+	if ((old_goptions & ~new_goptions) != 0
+		&& ACLITEM_GET_IDTYPE(*mod_aip) == ACL_IDTYPE_UID)
+		new_acl = recursive_revoke(new_acl, mod_aip->ai_grantee,
+								   (old_goptions & ~new_goptions),
+								   behavior);
 
 	return new_acl;
 }
@@ -744,8 +758,8 @@ aclremove(PG_FUNCTION_ARGS)
 				new_num;
 
 	/* These checks for null input should be dead code, but... */
-	if (!old_acl || ACL_NUM(old_acl) < 1)
-		old_acl = allocacl(1);
+	if (!old_acl || ACL_NUM(old_acl) < 0)
+		old_acl = allocacl(0);
 	if (!mod_aip)
 	{
 		new_acl = allocacl(ACL_NUM(old_acl));
@@ -773,27 +787,14 @@ aclremove(PG_FUNCTION_ARGS)
 		new_num = old_num - 1;
 		new_acl = allocacl(new_num);
 		new_aip = ACL_DAT(new_acl);
-		if (dst == 0)
-		{						/* start */
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_EXCEPTION),
-					 errmsg("aclitem for public may not be removed")));
-		}
-		else if (dst == old_num - 1)
-		{						/* end */
-			memcpy((char *) new_aip,
-				   (char *) old_aip,
-				   new_num * sizeof(AclItem));
-		}
-		else
-		{						/* middle */
+		if (dst > 0)
 			memcpy((char *) new_aip,
 				   (char *) old_aip,
 				   dst * sizeof(AclItem));
+		if (dst < new_num)
 			memcpy((char *) (new_aip + dst),
 				   (char *) (old_aip + dst + 1),
 				   (new_num - dst) * sizeof(AclItem));
-		}
 	}
 
 	PG_RETURN_ACL_P(new_acl);
@@ -839,7 +840,7 @@ makeaclitem(PG_FUNCTION_ARGS)
 
 	if (u_grantee == 0 && g_grantee == 0)
 	{
-		aclitem   ->ai_grantee = 0;
+		aclitem->ai_grantee = ACL_ID_WORLD;
 
 		ACLITEM_SET_IDTYPE(*aclitem, ACL_IDTYPE_WORLD);
 	}
@@ -851,18 +852,18 @@ makeaclitem(PG_FUNCTION_ARGS)
 	}
 	else if (u_grantee != 0)
 	{
-		aclitem   ->ai_grantee = u_grantee;
+		aclitem->ai_grantee = u_grantee;
 
 		ACLITEM_SET_IDTYPE(*aclitem, ACL_IDTYPE_UID);
 	}
-	else if (g_grantee != 0)
+	else /* (g_grantee != 0) */
 	{
-		aclitem   ->ai_grantee = g_grantee;
+		aclitem->ai_grantee = g_grantee;
 
 		ACLITEM_SET_IDTYPE(*aclitem, ACL_IDTYPE_GID);
 	}
 
-	aclitem   ->ai_grantor = grantor;
+	aclitem->ai_grantor = grantor;
 
 	ACLITEM_SET_PRIVS(*aclitem, priv);
 	if (goption)

src/include/utils/acl.h

@@ -7,16 +7,18 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: acl.h,v 1.62 2003/08/17 19:58:06 tgl Exp $
+ * $Id: acl.h,v 1.63 2003/10/29 22:20:54 tgl Exp $
  *
  * NOTES
- *	  For backward-compatibility purposes we have to allow there
- *	  to be a null ACL in a pg_class tuple.  This will be defined as
- *	  meaning "default protection" (i.e., whatever acldefault() returns).
+ *	  An ACL array is simply an array of AclItems, representing the union
+ *	  of the privileges represented by the individual items.  A zero-length
+ *	  array represents "no privileges".  There are no assumptions about the
+ *	  ordering of the items, but we do expect that there are no two entries
+ *	  in the array with the same grantor and grantee.
  *
- *	  The AclItems in an ACL array are currently kept in sorted order.
- *	  Things will break hard if you change that without changing the
- *	  code wherever this is included.
+ *	  For backward-compatibility purposes we have to allow null ACL entries
+ *	  in system catalogs.  A null ACL will be treated as meaning "default
+ *	  protection" (i.e., whatever acldefault() returns).
  *-------------------------------------------------------------------------
  */
 #ifndef ACL_H
@@ -45,13 +47,16 @@ typedef uint32 AclMode;
 /*
  * AclItem
  *
+ * The IDTYPE included in ai_privs identifies the type of the grantee ID.
+ * The grantor ID currently must always be a user, never a group.  (FIXME)
+ *
  * Note: must be same size on all platforms, because the size is hardcoded
  * in the pg_type.h entry for aclitem.
  */
 typedef struct AclItem
 {
-	AclId		ai_grantee;		/* ID that this item applies to */
-	AclId		ai_grantor;
+	AclId		ai_grantee;		/* ID that this item grants privs to */
+	AclId		ai_grantor;		/* grantor of privs (always a user id) */
 	AclMode		ai_privs;		/* AclIdType plus privilege bits */
 } AclItem;
 
@@ -61,20 +66,25 @@ typedef struct AclItem
  * and the lower 15 bits are the actual privileges.
  */
 #define ACLITEM_GET_PRIVS(item)    ((item).ai_privs & 0x7FFF)
-#define ACLITEM_GET_GOPTIONS(item) (((item).ai_privs >> 15)  & 0x7FFF)
+#define ACLITEM_GET_GOPTIONS(item) (((item).ai_privs >> 15) & 0x7FFF)
 #define ACLITEM_GET_IDTYPE(item)   ((item).ai_privs >> 30)
 
-#define ACL_GRANT_OPTION_FOR(privs) (((privs) & 0x7FFF) << 15)
+#define ACL_GRANT_OPTION_FOR(privs) (((AclMode) (privs) & 0x7FFF) << 15)
 
 #define ACLITEM_SET_PRIVS(item,privs) \
-  ((item).ai_privs = (ACLITEM_GET_IDTYPE(item)<<30) | (ACLITEM_GET_GOPTIONS(item)<<15) | ((privs) & 0x7FFF))
+  ((item).ai_privs = ((item).ai_privs & ~((AclMode) 0x7FFF)) | \
+					 ((AclMode) (privs) & 0x7FFF))
 #define ACLITEM_SET_GOPTIONS(item,goptions) \
-  ((item).ai_privs = (ACLITEM_GET_IDTYPE(item)<<30) | (((goptions) & 0x7FFF) << 15) | ACLITEM_GET_PRIVS(item))
+  ((item).ai_privs = ((item).ai_privs & ~(((AclMode) 0x7FFF) << 15)) | \
+					 (((AclMode) (goptions) & 0x7FFF) << 15))
 #define ACLITEM_SET_IDTYPE(item,idtype) \
-  ((item).ai_privs = ((idtype)<<30) | (ACLITEM_GET_GOPTIONS(item)<<15) | ACLITEM_GET_PRIVS(item))
+  ((item).ai_privs = ((item).ai_privs & ~(((AclMode) 0x03) << 30)) | \
+					 (((AclMode) (idtype) & 0x03) << 30))
 
 #define ACLITEM_SET_PRIVS_IDTYPE(item,privs,goption,idtype) \
-  ((item).ai_privs = ((privs) & 0x7FFF) |(((goption) & 0x7FFF) << 15) | ((idtype) << 30))
+  ((item).ai_privs = ((AclMode) (privs) & 0x7FFF) | \
+					 (((AclMode) (goption) & 0x7FFF) << 15) | \
+					 ((AclMode) (idtype) << 30))
 
 
 /*