getting confused parsing ACLITEMS...
This is the situation, I create a user called "
test=# create user """";
CREATE USER
test=# drop user """";
DROP USER
test=# create user """";
CREATE USER
test=# create table temp(a int4);
CREATE TABLE
test=# grant select on temp to """";
GRANT
test=# \dp temp
Access privileges for database "test"
Schema | Table | Access privileges
--------+-------+-----------------------------------------------------
public | temp | {chriskl=a*r*w*d*R*x*t*/chriskl,"\"\"\"=r/chriskl"}
(1 row)
OK, so the second aclitem is:
"\"\"\"=r/chriskl"
So how on earth does that quoting work????
If I remove the first and last quote (assuming the whole thing is quoted),
unescape it, then I get this:
"""=r/chriskl
Which I cannot parse, as """ doens't mean anything! I think that the second
aclitem should appear like this;
"\"\\\"\"=r/chriskl"
Which will (after removing string quotes and unescaping), reduce to this:
"\""=r/chriskl
I notice that it doesn't confuse postgres itself though:
test=# revoke select on temp from """";
REVOKE
test=# \dp temp
Access privileges for database "test"
Schema | Table | Access privileges
--------+-------+----------------------------------
public | temp | {chriskl=a*r*w*d*R*x*t*/chriskl}
(1 row)
So is there a bug here?
Chris
The situation seems to be a bug that this patch would address. It seems to
me that when a username is considered unsafe due to containing double
quotes, the double quotes should be escaped (and the backslashes)!
Does this look alright?
Chris
Index: src/backend/utils/adt/acl.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/adt/acl.c,v
retrieving revision 1.94
diff -c -r1.94 acl.c
*** src/backend/utils/adt/acl.c 4 Aug 2003 02:40:04 -0000 1.94
--- src/backend/utils/adt/acl.c 8 Aug 2003 09:03:19 -0000
***************
*** 124,131 ****
}
if (!safe)
*p++ = '"';
! for (src = s; *src; src++)
*p++ = *src;
if (!safe)
*p++ = '"';
*p = '\0';
--- 124,134 ----
}
if (!safe)
*p++ = '"';
! for (src = s; *src; src++) {
! if (!safe && (*src == '"' || *src == '\\'))
! *p++ = '\\';
*p++ = *src;
+ }
if (!safe)
*p++ = '"';
*p = '\0';
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
The situation seems to be a bug that this patch would address. It seems to
me that when a username is considered unsafe due to containing double
quotes, the double quotes should be escaped (and the backslashes)!
Does this look alright?
I would personally say that the convention ought to match what is done
in quoted identifiers, namely: " becomes "", backslash is not special
and therefore doesn't need anything.
More to the point, this is highly incomplete... you did not teach the
adjacent getid routine about this, and there is code in (at least)
pg_dump.c that knows the quoting conventions used here.
regards, tom lane
OK,
So if you agree that there is a quoting problem,and you don't mind
breaking backwards compatibility for it, I'll do a complete patch...
Chris
On Fri, 8 Aug 2003, Tom Lane wrote:
Show quoted text
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
The situation seems to be a bug that this patch would address. It seems to
me that when a username is considered unsafe due to containing double
quotes, the double quotes should be escaped (and the backslashes)!Does this look alright?
I would personally say that the convention ought to match what is done
in quoted identifiers, namely: " becomes "", backslash is not special
and therefore doesn't need anything.More to the point, this is highly incomplete... you did not teach the
adjacent getid routine about this, and there is code in (at least)
pg_dump.c that knows the quoting conventions used here.regards, tom lane
More to the point, this is highly incomplete... you did not teach the
adjacent getid routine about this, and there is code in (at least)
pg_dump.c that knows the quoting conventions used here.
Hang on - those routines can parse the acls just fine? How? How do they
handle usernames with equals signs in them (my major prob). How can it
work at all?
Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
So if you agree that there is a quoting problem,and you don't mind
breaking backwards compatibility for it, I'll do a complete patch...
I don't see any backwards-compatibility issue, because usernames
containing double quotes just plain don't work in past releases;
we've never before bothered to have a complete quoting solution
in ACLs.
regards, tom lane
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
More to the point, this is highly incomplete... you did not teach the
adjacent getid routine about this, and there is code in (at least)
pg_dump.c that knows the quoting conventions used here.
Hang on - those routines can parse the acls just fine? How? How do they
handle usernames with equals signs in them (my major prob). How can it
work at all?
IIRC, the present code assumes that usernames won't contain embedded
doublequotes. I did recently fix pg_dump to work in cases that it
wouldn't have handled before, including embedded equals. (BTW, my
mistake above: the pg_dump code is not in pg_dump.c, but in dumputils.c.
Look at copyAclUserName in particular.)
regards, tom lane
Tom Lane wrote:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
So if you agree that there is a quoting problem,and you don't mind
breaking backwards compatibility for it, I'll do a complete patch...I don't see any backwards-compatibility issue, because usernames
containing double quotes just plain don't work in past releases;
we've never before bothered to have a complete quoting solution
in ACLs.
Is it useful to allow these special chars at all? Seems this creates a
lot of work, and most admins will probably stick to "normal" user names
anyway.
Regards,
Andreas
Andreas Pflug <pgadmin@pse-consulting.de> writes:
Is it useful to allow these special chars at all? Seems this creates a
lot of work, and most admins will probably stick to "normal" user names
anyway.
Well, the reason it's been left unfixed for so long is exactly that it
didn't seem pressing. But if Chris wants to do the work, I won't stand
in his way ...
regards, tom lane
Of course, now I've just gone to some trouble to accomodate funky
characters in user and dbnames in logging I'd have to kill him ...
:-)
Seriously, I think there's a good case for banning a few characters in
at least some names - like []<>'"~#*|\ , say
andrew
Tom Lane wrote:
Show quoted text
Andreas Pflug <pgadmin@pse-consulting.de> writes:
Is it useful to allow these special chars at all? Seems this creates a
lot of work, and most admins will probably stick to "normal" user names
anyway.Well, the reason it's been left unfixed for so long is exactly that it
didn't seem pressing. But if Chris wants to do the work, I won't stand
in his way ...
I don't see any backwards-compatibility issue, because usernames
containing double quotes just plain don't work in past releases;
we've never before bothered to have a complete quoting solution
in ACLs.Is it useful to allow these special chars at all? Seems this creates a
lot of work, and most admins will probably stick to "normal" user names
anyway.
It's the principle of the thing :) Also, I don't know how non-english
speakers treat their double quote characers...
Chris
Seriously, I think there's a good case for banning a few characters in
at least some names - like []<>'"~#*|\ , say
Why? They're allowed in all other identifiers. And what if someone
already has a database full of usernames with those chars? They wouldn't
be able to load their dump properly...
Chris
Show quoted text
andrew
Tom Lane wrote:
Andreas Pflug <pgadmin@pse-consulting.de> writes:
Is it useful to allow these special chars at all? Seems this creates a
lot of work, and most admins will probably stick to "normal" user names
anyway.Well, the reason it's been left unfixed for so long is exactly that it
didn't seem pressing. But if Chris wants to do the work, I won't stand
in his way ...---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
I know. It just makes a few things a pain if you can't say "I know this
character can't be part of that".
Nevermind. Just wishful thinking. I'll shut up now.
andrew
Christopher Kings-Lynne wrote:
Show quoted text
Seriously, I think there's a good case for banning a few characters in
at least some names - like []<>'"~#*|\ , sayWhy? They're allowed in all other identifiers. And what if someone
already has a database full of usernames with those chars? They wouldn't
be able to load their dump properly...Chris
I believe Tom just applied this. Thanks.
---------------------------------------------------------------------------
Christopher Kings-Lynne wrote:
The situation seems to be a bug that this patch would address. It seems to
me that when a username is considered unsafe due to containing double
quotes, the double quotes should be escaped (and the backslashes)!Does this look alright?
Chris
Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/adt/acl.c,v retrieving revision 1.94 diff -c -r1.94 acl.c *** src/backend/utils/adt/acl.c 4 Aug 2003 02:40:04 -0000 1.94 --- src/backend/utils/adt/acl.c 8 Aug 2003 09:03:19 -0000 *************** *** 124,131 **** } if (!safe) *p++ = '"'; ! for (src = s; *src; src++) *p++ = *src; if (!safe) *p++ = '"'; *p = '\0'; --- 124,134 ---- } if (!safe) *p++ = '"'; ! for (src = s; *src; src++) { ! if (!safe && (*src == '"' || *src == '\\')) ! *p++ = '\\'; *p++ = *src; + } if (!safe) *p++ = '"'; *p = '\0';---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073