diff options
| -rw-r--r-- | src/backend/utils/adt/acl.c | 33 | ||||
| -rw-r--r-- | src/test/regress/expected/privileges.out | 20 | ||||
| -rw-r--r-- | src/test/regress/sql/privileges.sql | 8 |
3 files changed, 53 insertions, 8 deletions
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index a44428da7ae..9489e01193a 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -116,6 +116,22 @@ static void RoleMembershipCacheCallback(Datum arg, int cacheid, uint32 hashvalue /* + * Test whether an identifier char can be left unquoted in ACLs. + * + * Formerly, we used isalnum() even on non-ASCII characters, resulting in + * unportable behavior. To ensure dump compatibility with old versions, + * we now treat high-bit-set characters as always requiring quoting during + * putid(), but getid() will always accept them without quotes. + */ +static inline bool +is_safe_acl_char(unsigned char c, bool is_getid) +{ + if (IS_HIGHBIT_SET(c)) + return is_getid; + return isalnum(c) || c == '_'; +} + +/* * getid * Consumes the first alphanumeric string (identifier) found in string * 's', ignoring any leading white space. If it finds a double quote @@ -137,21 +153,22 @@ getid(const char *s, char *n) while (isspace((unsigned char) *s)) s++; - /* This code had better match what putid() does, below */ for (; *s != '\0' && - (isalnum((unsigned char) *s) || - *s == '_' || - *s == '"' || - in_quotes); + (in_quotes || *s == '"' || is_safe_acl_char(*s, true)); s++) { if (*s == '"') { + if (!in_quotes) + { + in_quotes = true; + continue; + } /* safe to look at next char (could be '\0' though) */ if (*(s + 1) != '"') { - in_quotes = !in_quotes; + in_quotes = false; continue; } /* it's an escaped double quote; skip the escaping char */ @@ -185,10 +202,10 @@ putid(char *p, const char *s) const char *src; bool safe = true; + /* Detect whether we need to use double quotes */ for (src = s; *src; src++) { - /* This test had better match what getid() does, above */ - if (!isalnum((unsigned char) *src) && *src != '_') + if (!is_safe_acl_char(*src, false)) { safe = false; break; diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index bd67db5d180..7c9aa84a585 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2076,6 +2076,26 @@ SELECT has_table_privilege('regress_priv_user1', 'testns.acltest1', 'INSERT'); - ALTER DEFAULT PRIVILEGES FOR ROLE regress_priv_user1 REVOKE EXECUTE ON FUNCTIONS FROM public; ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error ERROR: cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS +-- Test quoting and dequoting of user names in ACLs +CREATE ROLE "regress_""quoted"; +SELECT makeaclitem('regress_"quoted'::regrole, 'regress_"quoted'::regrole, + 'SELECT', TRUE); + makeaclitem +------------------------------------------ + "regress_""quoted"=r*/"regress_""quoted" +(1 row) + +SELECT '"regress_""quoted"=r*/"regress_""quoted"'::aclitem; + aclitem +------------------------------------------ + "regress_""quoted"=r*/"regress_""quoted" +(1 row) + +SELECT '""=r*/""'::aclitem; -- used to be misparsed as """" +ERROR: a name must follow the "/" sign +LINE 1: SELECT '""=r*/""'::aclitem; + ^ +DROP ROLE "regress_""quoted"; -- -- Testing blanket default grants is very hazardous since it might change -- the privileges attached to objects created by concurrent regression tests. diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 94272a1cded..ab42f511d74 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1260,6 +1260,14 @@ ALTER DEFAULT PRIVILEGES FOR ROLE regress_priv_user1 REVOKE EXECUTE ON FUNCTIONS ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error +-- Test quoting and dequoting of user names in ACLs +CREATE ROLE "regress_""quoted"; +SELECT makeaclitem('regress_"quoted'::regrole, 'regress_"quoted'::regrole, + 'SELECT', TRUE); +SELECT '"regress_""quoted"=r*/"regress_""quoted"'::aclitem; +SELECT '""=r*/""'::aclitem; -- used to be misparsed as """" +DROP ROLE "regress_""quoted"; + -- -- Testing blanket default grants is very hazardous since it might change -- the privileges attached to objects created by concurrent regression tests. |