Tom Lane <[email protected]> Wednesday 23 February 2011 16:19:27
> rsmogura <[email protected]> writes:
> > On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
> >> ... But my question isn't about that; it's about
> >> why aclitem should be considered a first-class citizen. It makes me
> >> uncomfortable that client apps are looking at it at all, because any
> >> that do are bound to get broken in the future, even assuming that
> >> they get the right answers today. I wonder how many such clients are up
> >> to speed for per-column privileges and non-constant default privileges
> >> for instance. And sepgsql is going to cut them off at the knees.
> >>
> > Technically, at eye glance, I didn't seen in sepgsql modifications to
> > acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
> > some way to present access rights to administrator it may use own model,
> > or aclitem, too.
>
> You're missing the point, which is that the current internal
> representation of aclitem could change drastically to support future
> feature improvements in the area of privileges. It has already changed
> significantly in the past (we didn't use to have WITH GRANT OPTION).
> If we had to add a field, for instance, a binary representation would
> simply be broken, as clients would have difficulty telling how to
> interpret it as soon as there was more than one possible format.
> Text representations are typically a bit more extensible.
>
> regards, tom lane
Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
mask, as well definition is more general then def of PGSQL. In any way it
require that rights mades bit array.
Still I tested only aclitemsend.
Btw, Is it possible and needed to add group byte, indicating that grantee is
group or user?
Regards,
Radek
diff --git a/.gitignore b/.gitignore
index 1be11e8..0d594f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,5 @@ objfiles.txt
/GNUmakefile
/config.log
/config.status
+/nbproject/private/
+/nbproject
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 691ba3b..c25c0fd 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -33,6 +33,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/syscache.h"
+#include "libpq/pqformat.h"
typedef struct
@@ -78,6 +79,10 @@ static void putid(char *p, const char *s);
static Acl *allocacl(int n);
static void check_acl(const Acl *acl);
static const char *aclparse(const char *s, AclItem *aip);
+
+/** Assigns default grantor and send warning. */
+static void aclitem_assign_default_grantor(AclItem *aip);
+
static bool aclitem_match(const AclItem *a1, const AclItem *a2);
static int aclitemComparator(const void *arg1, const void *arg2);
static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
@@ -209,6 +214,14 @@ putid(char *p, const char *s)
*p = '\0';
}
+/** Assigns default grantor and send warning. */
+void aclitem_assign_default_grantor(AclItem *aip) {
+ aip->ai_grantor = BOOTSTRAP_SUPERUSERID;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_GRANTOR),
+ errmsg("defaulting grantor to user ID %u",
+ BOOTSTRAP_SUPERUSERID)));
+}
/*
* aclparse
* Consumes and parses an ACL specification of the form:
@@ -343,11 +356,7 @@ aclparse(const char *s, AclItem *aip)
}
else
{
- aip->ai_grantor = BOOTSTRAP_SUPERUSERID;
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_GRANTOR),
- errmsg("defaulting grantor to user ID %u",
- BOOTSTRAP_SUPERUSERID)));
+ aclitem_assign_default_grantor(aip);
}
ACLITEM_SET_PRIVS_GOPTIONS(*aip, privs, goption);
@@ -643,6 +652,163 @@ aclitemout(PG_FUNCTION_ARGS)
PG_RETURN_CSTRING(out);
}
+/** Do binary read of aclitem. Input format is same as {@link aclitem_recv}, but
+ * special algorithm is used to determine grantee's and grantor's OID. The reason
+ * is to keep backward "information" compatiblity with text mode - typical
+ * client (which gets instructions from user)
+ * may be much more interested in sending grantee and grantors name then
+ * OID. Detailed rule is as follow:<br/>
+ * If message has no name and names' length then
+ * use passed OIDs (message may be truncated, we accept this,
+ * but both, two last fields must be not present).<br/>
+ * If grantee's name len or grantor's name len is {@code -1} then use respecitve
+ * OIDs.<br/>
+ * If name length is not {@code -1} then find OID for given part, and
+ * ensure that respective OID is {@code 0} or is equal to found OID.</br>
+ * If grantor's OID is {@code 0} and grantor's name lenght is {@code -1} or
+ * truncated then assign superuser as grantor.
+ */
+Datum
+aclitemrecv(PG_FUNCTION_ARGS) {
+ StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
+ AclItem *aip;
+ int gRawLen;
+ char *gVal = NULL;
+ int4 gValLen;
+ Oid gOid;
+ int2 numberOfAcls;
+ int4 mask;
+
+ aip = (AclItem *) palloc(sizeof(AclItem));
+ aip->ai_grantee = pq_getmsgint(buf, 4);
+ aip->ai_grantor = pq_getmsgint(buf, 4);
+ numberOfAcls = pq_getmsgint(buf, 2);
+ if (numberOfAcls != N_ACL_RIGHTS * 2) {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
+ errmsg("grantee's OID is not 0 and passed grantee's name and grantee's OID doesn't match")));
+ aip->ai_privs = ACL_NO_RIGHTS;
+ PG_RETURN_ACLITEM_P(aip);
+ }
+
+ aip->ai_privs = pq_getmsgint(buf, 4);
+ mask = pq_getmsgint(buf, 4);
+
+ /* Client has passed names. */
+ if (buf->cursor < buf->len) {
+ /* Read grantee. */
+ gRawLen = pq_getmsgint(buf, 4);
+ if (gRawLen != -1) {
+ gVal = pq_getmsgtext(buf, gRawLen, &gValLen);
+ gOid = get_role_oid(gVal, false);
+ if (aip->ai_grantee != 0 && aip->ai_grantee != gOid) {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
+ errmsg("grantee's OID is not 0 and passed grantee's name and grantee's OID doesn't match")));
+
+ }
+ }
+
+ /* Read grantor. */
+ gRawLen = pq_getmsgint(buf, 4);
+ if (gRawLen != -1) {
+ gVal = pq_getmsgtext(buf, gRawLen, &gValLen);
+ gOid = get_role_oid(gVal, false);
+ if (aip->ai_grantor != 0 && aip->ai_grantor != gOid) {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
+ errmsg("grantor's OID is not 0 and passed grantor's name and grantor's OID doesn't match")));
+ }
+ }else {
+ gVal = NULL;
+ }
+ }
+
+ if (aip->ai_grantor == 0 && gVal == NULL)
+ aclitem_assign_default_grantor(aip);
+
+ PG_RETURN_ACLITEM_P(aip);
+}
+/**
+ * The output format is as follows:
+ * <ol>
+ * <li>grantee's oid</li>
+ * <li>grantor's oid</li>
+ * <li>number of rights (describes priviliges set version) (short)</li>
+ * <li>rights - N-bit number</li>
+ * <li>mask (reserved for future usage) - N-bit number</li>
+ * <li>grentee's name length</li>
+ * <li>grantee's name</li>
+ * <li>grantor's name length</li>
+ * <li>grantor's name</li>
+ * </ol>
+ * "Number of rights" - describes how many rights PSQL has defined. Each right
+ * has internal number <i>n</i> and is represented by bit 1 set on <i>n</i>-th
+ * internal positon. Currently we have 24 rigths (12 base and 12 grantor)<br/>
+ * N is lowest number such that {@code N%8 == 0 && N >= 32 && N >= "Number of rights"
+ * <br/>
+ * <b>Backend implementation notice</b>
+ * Currently, when I wrote this
+ * rights indexed from 1-12 represents "base" privilege, rights 16-29 represents
+ * privilige to grant base privilige. So, rights 13-15, 30-32 are reserved for
+ * future usage.
+ * <br/>
+ * Magic lenght for grantee's or grantor's name -1 means that respecitive
+ * is unnamed, which will be, for example, for {@link ACL_ID_PUBLIC}.
+ * <br/>
+ * Last elements are used to pass "usefull" information to client, so it
+ * don't need to query for those names, actually client may be more
+ * interested in getting those names, then OIDs. It will keep backward
+ * compatibility with text mode, as well.
+ */
+Datum
+aclitemsend(PG_FUNCTION_ARGS)
+{
+ AclItem *aip = PG_GETARG_ACLITEM_P(0);
+ char *oidName;
+ HeapTuple htup;
+ StringInfoData buf;
+ //Here we use simplification of documentation, because of we have < 16 priviliges
+ pq_begintypsend(&buf);
+ pq_sendint(&buf, aip->ai_grantee, 4);
+ pq_sendint(&buf, aip->ai_grantor, 4);
+ //Version
+ pq_sendint(&buf, N_ACL_RIGHTS*2, 2);
+ pq_sendint(&buf, aip->ai_privs, 4);
+ //Mask currently all 1
+ pq_sendint(&buf, 0xFFffFFff, 4);
+ if (N_ACL_RIGHTS > 16) {
+ /* We sends rights as long (16 bit), but if in future number
+ * of rights will increase we got problem.
+ */
+ }
+
+ /** Append names at the end. */
+ if (aip->ai_grantee != ACL_ID_PUBLIC) {
+ htup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(aip->ai_grantee));
+ if (HeapTupleIsValid(htup)) {
+ oidName = NameStr(((Form_pg_authid) GETSTRUCT(htup))->rolname);
+ pq_sendcountedtext(&buf, oidName, strlen(oidName), false);
+ ReleaseSysCache(htup);
+ }else {
+ pq_sendint(&buf,-1, 4);
+ }
+ }else {
+ pq_sendint(&buf, -1, 4);
+ }
+
+ htup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(aip->ai_grantor));
+ if (HeapTupleIsValid(htup)) {
+ oidName = NameStr(((Form_pg_authid) GETSTRUCT(htup))->rolname);
+ pq_sendcountedtext(&buf, oidName, strlen(oidName), false);
+ ReleaseSysCache(htup);
+ }
+ else {
+ pq_sendint(&buf, -1, 4);
+ }
+
+ PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
+}
/*
* aclitem_match
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 8c940bb..c56d8f8 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4230,6 +4230,10 @@ DATA(insert OID = 3120 ( void_recv PGNSP PGUID 12 1 0 0 f f f t f i 1 0 22
DESCR("I/O");
DATA(insert OID = 3121 ( void_send PGNSP PGUID 12 1 0 0 f f f t f i 1 0 17 "2278" _null_ _null_ _null_ _null_ void_send _null_ _null_ _null_ ));
DESCR("I/O");
+DATA(insert OID = 3122 ( aclitemrecv PGNSP PGUID 12 1 0 0 f f f t f i 1 0 1033 "2281" _null_ _null_ _null_ _null_ aclitemrecv _null_ _null_ _null_ ));
+DESCR("I/O");
+DATA(insert OID = 3123 ( aclitemsend PGNSP PGUID 12 1 0 0 f f f t f i 1 0 17 "1033" _null_ _null_ _null_ _null_ aclitemsend _null_ _null_ _null_ ));
+DESCR("I/O");
/* System-view support functions with pretty-print option */
DATA(insert OID = 2504 ( pg_get_ruledef PGNSP PGUID 12 1 0 0 f f f t f s 2 0 25 "26 16" _null_ _null_ _null_ _null_ pg_get_ruledef_ext _null_ _null_ _null_ ));
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index 9baed6c..f89ed44 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -462,7 +462,7 @@ DATA(insert OID = 1023 ( _abstime PGNSP PGUID -1 f b A f t \054 0 702 0 array_
DATA(insert OID = 1024 ( _reltime PGNSP PGUID -1 f b A f t \054 0 703 0 array_in array_out array_recv array_send - - - i x f 0 -1 0 0 _null_ _null_ ));
DATA(insert OID = 1025 ( _tinterval PGNSP PGUID -1 f b A f t \054 0 704 0 array_in array_out array_recv array_send - - - i x f 0 -1 0 0 _null_ _null_ ));
DATA(insert OID = 1027 ( _polygon PGNSP PGUID -1 f b A f t \054 0 604 0 array_in array_out array_recv array_send - - - d x f 0 -1 0 0 _null_ _null_ ));
-DATA(insert OID = 1033 ( aclitem PGNSP PGUID 12 f b U f t \054 0 0 1034 aclitemin aclitemout - - - - - i p f 0 -1 0 0 _null_ _null_ ));
+DATA(insert OID = 1033 ( aclitem PGNSP PGUID 12 f b U f t \054 0 0 1034 aclitemin aclitemout aclitemrecv aclitemsend - - - i p f 0 -1 0 0 _null_ _null_ ));
DESCR("access control list");
#define ACLITEMOID 1033
DATA(insert OID = 1034 ( _aclitem PGNSP PGUID -1 f b A f t \054 0 1033 0 array_in array_out array_recv array_send - - - i x f 0 -1 0 0 _null_ _null_ ));
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 1e9cf7f..1ca959d 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -239,6 +239,8 @@ extern void initialize_acl(void);
*/
extern Datum aclitemin(PG_FUNCTION_ARGS);
extern Datum aclitemout(PG_FUNCTION_ARGS);
+extern Datum aclitemrecv(PG_FUNCTION_ARGS);
+extern Datum aclitemsend(PG_FUNCTION_ARGS);
extern Datum aclinsert(PG_FUNCTION_ARGS);
extern Datum aclremove(PG_FUNCTION_ARGS);
extern Datum aclcontains(PG_FUNCTION_ARGS);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
