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


Fri, 8 Aug 2003 16:31:01 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
getting confused parsing ACLITEMS...
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';




Fri, 8 Aug 2003 17:09:09 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
Re: getting confused parsing ACLITEMS...
"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

Fri, 08 Aug 2003 11:00:52 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: getting confused parsing ACLITEMS... 
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:

> "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
>


Sat, 9 Aug 2003 22:39:54 +0800 (WST)
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
Re: getting confused parsing ACLITEMS... 
> 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



Sat, 9 Aug 2003 22:41:04 +0800 (WST)
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
Re: getting confused parsing ACLITEMS... 
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

Sat, 09 Aug 2003 10:55:43 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: getting confused parsing ACLITEMS... 
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

Sat, 09 Aug 2003 11:00:41 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: getting confused parsing ACLITEMS... 
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


Sat, 09 Aug 2003 20:43:33 +0200
Andreas Pflug <pgadmin@pse-consulting.de>
Re: getting confused parsing ACLITEMS...
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

Sat, 09 Aug 2003 18:53:07 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: getting confused parsing ACLITEMS... 
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:

>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 ...
>
>
>  
>


Sat, 09 Aug 2003 19:14:33 -0400
Andrew Dunstan <andrew@dunslane.net>
Re: getting confused parsing ACLITEMS...
> >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



Sun, 10 Aug 2003 11:24:39 +0800 (WST)
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
Re: getting confused parsing ACLITEMS...
> 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

> 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)
>


Sun, 10 Aug 2003 11:26:57 +0800 (WST)
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
Re: getting confused parsing ACLITEMS...

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:

>>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
>  
>


Sat, 09 Aug 2003 23:43:32 -0400
Andrew Dunstan <andrew@dunslane.net>
Re: getting confused parsing ACLITEMS...

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?
> 
>                http://archives.postgresql.org
> 

-- 
  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

Thu, 14 Aug 2003 17:56:06 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: getting confused parsing ACLITEMS...