keywords in initdb are case-sensitive?
In miracee's review of Peter's patch for new -A options in initdb (in
commitfest app only), it is noted that pg_hba.conf keyword parsing is
done in a case sensitive manner. So if you write "Trust" rather than
"trust", it's not recognized.
This seemed pretty nonsensical to me, and it's not documented, so I came
up with the trivial attached patch.
Comparisons to user and database names and the like are unchanged and
thus require matching case.
Thoughts?
--
Álvaro Herrera <alvherre@alvh.no-ip.org>
Attachments:
hba-casecmp.patchapplication/octet-stream; name=hba-casecmp.patchDownload
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 1dadafc..c8d528c 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -52,7 +52,7 @@ typedef struct check_network_data
} check_network_data;
-#define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0)
+#define token_is_keyword(t, k) (!t->quoted && pg_strcasecmp(t->string, k) == 0)
#define token_matches(t, k) (strcmp(t->string, k) == 0)
/*
@@ -844,7 +844,7 @@ parse_hba_line(List *line, int line_num)
return NULL;
}
token = linitial(tokens);
- if (strcmp(token->string, "local") == 0)
+ if (pg_strcasecmp(token->string, "local") == 0)
{
#ifdef HAVE_UNIX_SOCKETS
parsedline->conntype = ctLocal;
@@ -857,9 +857,9 @@ parse_hba_line(List *line, int line_num)
return NULL;
#endif
}
- else if (strcmp(token->string, "host") == 0 ||
- strcmp(token->string, "hostssl") == 0 ||
- strcmp(token->string, "hostnossl") == 0)
+ else if (pg_strcasecmp(token->string, "host") == 0 ||
+ pg_strcasecmp(token->string, "hostssl") == 0 ||
+ pg_strcasecmp(token->string, "hostnossl") == 0)
{
if (token->string[4] == 's') /* "hostssl" */
@@ -1144,35 +1144,35 @@ parse_hba_line(List *line, int line_num)
token = linitial(tokens);
unsupauth = NULL;
- if (strcmp(token->string, "trust") == 0)
+ if (pg_strcasecmp(token->string, "trust") == 0)
parsedline->auth_method = uaTrust;
- else if (strcmp(token->string, "ident") == 0)
+ else if (pg_strcasecmp(token->string, "ident") == 0)
parsedline->auth_method = uaIdent;
- else if (strcmp(token->string, "peer") == 0)
+ else if (pg_strcasecmp(token->string, "peer") == 0)
parsedline->auth_method = uaPeer;
- else if (strcmp(token->string, "password") == 0)
+ else if (pg_strcasecmp(token->string, "password") == 0)
parsedline->auth_method = uaPassword;
- else if (strcmp(token->string, "krb5") == 0)
+ else if (pg_strcasecmp(token->string, "krb5") == 0)
#ifdef KRB5
parsedline->auth_method = uaKrb5;
#else
unsupauth = "krb5";
#endif
- else if (strcmp(token->string, "gss") == 0)
+ else if (pg_strcasecmp(token->string, "gss") == 0)
#ifdef ENABLE_GSS
parsedline->auth_method = uaGSS;
#else
unsupauth = "gss";
#endif
- else if (strcmp(token->string, "sspi") == 0)
+ else if (pg_strcasecmp(token->string, "sspi") == 0)
#ifdef ENABLE_SSPI
parsedline->auth_method = uaSSPI;
#else
unsupauth = "sspi";
#endif
- else if (strcmp(token->string, "reject") == 0)
+ else if (pg_strcasecmp(token->string, "reject") == 0)
parsedline->auth_method = uaReject;
- else if (strcmp(token->string, "md5") == 0)
+ else if (pg_strcasecmp(token->string, "md5") == 0)
{
if (Db_user_namespace)
{
@@ -1185,25 +1185,25 @@ parse_hba_line(List *line, int line_num)
}
parsedline->auth_method = uaMD5;
}
- else if (strcmp(token->string, "pam") == 0)
+ else if (pg_strcasecmp(token->string, "pam") == 0)
#ifdef USE_PAM
parsedline->auth_method = uaPAM;
#else
unsupauth = "pam";
#endif
- else if (strcmp(token->string, "ldap") == 0)
+ else if (pg_strcasecmp(token->string, "ldap") == 0)
#ifdef USE_LDAP
parsedline->auth_method = uaLDAP;
#else
unsupauth = "ldap";
#endif
- else if (strcmp(token->string, "cert") == 0)
+ else if (pg_strcasecmp(token->string, "cert") == 0)
#ifdef USE_SSL
parsedline->auth_method = uaCert;
#else
unsupauth = "cert";
#endif
- else if (strcmp(token->string, "radius") == 0)
+ else if (pg_strcasecmp(token->string, "radius") == 0)
parsedline->auth_method = uaRADIUS;
else
{
@@ -1383,7 +1383,7 @@ parse_hba_line(List *line, int line_num)
static bool
parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
{
- if (strcmp(name, "map") == 0)
+ if (pg_strcasecmp(name, "map") == 0)
{
if (hbaline->auth_method != uaIdent &&
hbaline->auth_method != uaPeer &&
@@ -1394,7 +1394,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
INVALID_AUTH_OPTION("map", gettext_noop("ident, peer, krb5, gssapi, sspi and cert"));
hbaline->usermap = pstrdup(val);
}
- else if (strcmp(name, "clientcert") == 0)
+ else if (pg_strcasecmp(name, "clientcert") == 0)
{
/*
* Since we require ctHostSSL, this really can never happen
@@ -1410,7 +1410,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
line_num, HbaFileName)));
return false;
}
- if (strcmp(val, "1") == 0)
+ if (pg_strcasecmp(val, "1") == 0)
{
if (!secure_loaded_verify_locations())
{
@@ -1438,25 +1438,25 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
hbaline->clientcert = false;
}
}
- else if (strcmp(name, "pamservice") == 0)
+ else if (pg_strcasecmp(name, "pamservice") == 0)
{
REQUIRE_AUTH_OPTION(uaPAM, "pamservice", "pam");
hbaline->pamservice = pstrdup(val);
}
- else if (strcmp(name, "ldaptls") == 0)
+ else if (pg_strcasecmp(name, "ldaptls") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldaptls", "ldap");
- if (strcmp(val, "1") == 0)
+ if (pg_strcasecmp(val, "1") == 0)
hbaline->ldaptls = true;
else
hbaline->ldaptls = false;
}
- else if (strcmp(name, "ldapserver") == 0)
+ else if (pg_strcasecmp(name, "ldapserver") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapserver", "ldap");
hbaline->ldapserver = pstrdup(val);
}
- else if (strcmp(name, "ldapport") == 0)
+ else if (pg_strcasecmp(name, "ldapport") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapport", "ldap");
hbaline->ldapport = atoi(val);
@@ -1470,42 +1470,42 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
return false;
}
}
- else if (strcmp(name, "ldapbinddn") == 0)
+ else if (pg_strcasecmp(name, "ldapbinddn") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapbinddn", "ldap");
hbaline->ldapbinddn = pstrdup(val);
}
- else if (strcmp(name, "ldapbindpasswd") == 0)
+ else if (pg_strcasecmp(name, "ldapbindpasswd") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapbindpasswd", "ldap");
hbaline->ldapbindpasswd = pstrdup(val);
}
- else if (strcmp(name, "ldapsearchattribute") == 0)
+ else if (pg_strcasecmp(name, "ldapsearchattribute") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapsearchattribute", "ldap");
hbaline->ldapsearchattribute = pstrdup(val);
}
- else if (strcmp(name, "ldapbasedn") == 0)
+ else if (pg_strcasecmp(name, "ldapbasedn") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapbasedn", "ldap");
hbaline->ldapbasedn = pstrdup(val);
}
- else if (strcmp(name, "ldapprefix") == 0)
+ else if (pg_strcasecmp(name, "ldapprefix") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapprefix", "ldap");
hbaline->ldapprefix = pstrdup(val);
}
- else if (strcmp(name, "ldapsuffix") == 0)
+ else if (pg_strcasecmp(name, "ldapsuffix") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapsuffix", "ldap");
hbaline->ldapsuffix = pstrdup(val);
}
- else if (strcmp(name, "krb_server_hostname") == 0)
+ else if (pg_strcasecmp(name, "krb_server_hostname") == 0)
{
REQUIRE_AUTH_OPTION(uaKrb5, "krb_server_hostname", "krb5");
hbaline->krb_server_hostname = pstrdup(val);
}
- else if (strcmp(name, "krb_realm") == 0)
+ else if (pg_strcasecmp(name, "krb_realm") == 0)
{
if (hbaline->auth_method != uaKrb5 &&
hbaline->auth_method != uaGSS &&
@@ -1513,18 +1513,18 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
INVALID_AUTH_OPTION("krb_realm", gettext_noop("krb5, gssapi and sspi"));
hbaline->krb_realm = pstrdup(val);
}
- else if (strcmp(name, "include_realm") == 0)
+ else if (pg_strcasecmp(name, "include_realm") == 0)
{
if (hbaline->auth_method != uaKrb5 &&
hbaline->auth_method != uaGSS &&
hbaline->auth_method != uaSSPI)
INVALID_AUTH_OPTION("include_realm", gettext_noop("krb5, gssapi and sspi"));
- if (strcmp(val, "1") == 0)
+ if (pg_strcasecmp(val, "1") == 0)
hbaline->include_realm = true;
else
hbaline->include_realm = false;
}
- else if (strcmp(name, "radiusserver") == 0)
+ else if (pg_strcasecmp(name, "radiusserver") == 0)
{
struct addrinfo *gai_result;
struct addrinfo hints;
@@ -1552,7 +1552,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
pg_freeaddrinfo_all(hints.ai_family, gai_result);
hbaline->radiusserver = pstrdup(val);
}
- else if (strcmp(name, "radiusport") == 0)
+ else if (pg_strcasecmp(name, "radiusport") == 0)
{
REQUIRE_AUTH_OPTION(uaRADIUS, "radiusport", "radius");
hbaline->radiusport = atoi(val);
@@ -1566,12 +1566,12 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
return false;
}
}
- else if (strcmp(name, "radiussecret") == 0)
+ else if (pg_strcasecmp(name, "radiussecret") == 0)
{
REQUIRE_AUTH_OPTION(uaRADIUS, "radiussecret", "radius");
hbaline->radiussecret = pstrdup(val);
}
- else if (strcmp(name, "radiusidentifier") == 0)
+ else if (pg_strcasecmp(name, "radiusidentifier") == 0)
{
REQUIRE_AUTH_OPTION(uaRADIUS, "radiusidentifier", "radius");
hbaline->radiusidentifier = pstrdup(val);
On Wed, Feb 1, 2012 at 11:45 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
In miracee's review of Peter's patch for new -A options in initdb (in
commitfest app only), it is noted that pg_hba.conf keyword parsing is
done in a case sensitive manner. So if you write "Trust" rather than
"trust", it's not recognized.This seemed pretty nonsensical to me, and it's not documented, so I came
up with the trivial attached patch.Comparisons to user and database names and the like are unchanged and
thus require matching case.Thoughts?
We have lots of things that are case-sensitive; I don't particularly
see why this one should be different.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Excerpts from Robert Haas's message of jue feb 02 11:39:29 -0300 2012:
On Wed, Feb 1, 2012 at 11:45 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
In miracee's review of Peter's patch for new -A options in initdb (in
commitfest app only), it is noted that pg_hba.conf keyword parsing is
done in a case sensitive manner. So if you write "Trust" rather than
"trust", it's not recognized.This seemed pretty nonsensical to me, and it's not documented, so I came
up with the trivial attached patch.Comparisons to user and database names and the like are unchanged and
thus require matching case.We have lots of things that are case-sensitive; I don't particularly
see why this one should be different.
Err, postgresql.conf processing is case insensitive, which is the most
closely related example. Are you saying we should make that case
sensitive as well? What I'm saying is that I see no good reason for
keyword comparison to be case sensitive here. We don't compare case on
SQL keywords either.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
[ adjusting thread title to have something to do with reality ]
Alvaro Herrera <alvherre@commandprompt.com> writes:
Excerpts from Robert Haas's message of jue feb 02 11:39:29 -0300 2012:
We have lots of things that are case-sensitive; I don't particularly
see why this one should be different.
Err, postgresql.conf processing is case insensitive, which is the most
closely related example. Are you saying we should make that case
sensitive as well? What I'm saying is that I see no good reason for
keyword comparison to be case sensitive here. We don't compare case on
SQL keywords either.
One thing I'm concerned about is that there are places in pg_hba.conf
where a token might be either a keyword or a user/group/database name.
If you start recognizing keywords case-insensitively, you could break
files that used to work, ie what was meant to be a name will now be
read as a keyword. Admittedly, the odds of that are not very large, but
they're not zero either. Given the entire lack of complaints about this
from the field, I'm inclined to think it's better to leave well enough
alone. We could add a documentation note if you feel a need for that.
regards, tom lane
On Thu, Feb 2, 2012 at 10:10 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
Err, postgresql.conf processing is case insensitive, which is the most
closely related example. Are you saying we should make that case
sensitive as well? What I'm saying is that I see no good reason for
keyword comparison to be case sensitive here. We don't compare case on
SQL keywords either.
In some sense this is just a religious question: UNIX commands are
case-sensitive (except on MacOS X, where I keep typing pg_Ctl start by
mistake and it lets me get away with it) and any UNIX purist worth his
salt will tell you that's the one true way. On the other hand,
ignoring case for comparison purposes has a long and distinguished
history, too, especially outside of computing. In this particular
case, I am mostly in favor of leaving it alone because I can't see any
real upside to changing it. Having multiple ways to spell the same
configuration setting, even when they differ only in case, complicates
the job of (for example) anyone who wants to write a pg_hba.conf file
parser: some files will be valid on 9.2 that weren't valid on 9.1, or
(as Tom points out) they might both be valid but mean subtly different
things in corner cases. If it were already case-sensitive, I would be
in favor of leaving that alone, too; there's just not enough upside to
justifying tinkering with it, IMV.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Import Notes
Resolved by subject fallback
On Feb 2, 2012 5:34 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
[ adjusting thread title to have something to do with reality ]
Alvaro Herrera <alvherre@commandprompt.com> writes:
Excerpts from Robert Haas's message of jue feb 02 11:39:29 -0300 2012:
We have lots of things that are case-sensitive; I don't particularly
see why this one should be different.Err, postgresql.conf processing is case insensitive, which is the most
closely related example. Are you saying we should make that case
sensitive as well? What I'm saying is that I see no good reason for
keyword comparison to be case sensitive here. We don't compare case on
SQL keywords either.One thing I'm concerned about is that there are places in pg_hba.conf
where a token might be either a keyword or a user/group/database name.
If you start recognizing keywords case-insensitively, you could break
files that used to work, ie what was meant to be a name will now be
read as a keyword. Admittedly, the odds of that are not very large, but
they're not zero either. Given the entire lack of complaints about this
from the field, I'm inclined to think it's better to leave well enough
alone. We could add a documentation note if you feel a need for that.
+1. I don't think I've heard a single complaint about this ever...
/Magnus