On Tue, May 19, 2020 at 09:32:15AM -0400, Tom Lane wrote: > Hm, I'm pretty certain that data_directory does not need this because > canonicalization is done elsewhere; the most that you could accomplish > there is to cause problems. Dunno about the rest.
Hmm. I missed that this is getting done in SelectConfigFiles() first by the postmaster so that's not necessary, which also does the work for hba_file and ident_file. config_file does not need that either as AbsoluteConfigLocation() does the same work via ParseConfigFile(). So perhaps we could add a comment or such about that? Attached is an idea. The rest is made of PromoteTriggerFile, pg_krb_server_keyfile, ssl_cert_file, ssl_key_file, ssl_ca_file, ssl_crl_file and ssl_dh_params_file where loaded values are taken as-is, so applying canonicalization would be helpful there, no? -- Michael
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2f3e0a70e0..99b66aa2a7 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3781,7 +3781,7 @@ static struct config_string ConfigureNamesString[] =
},
&PromoteTriggerFile,
"",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -3903,7 +3903,7 @@ static struct config_string ConfigureNamesString[] =
},
&pg_krb_server_keyfile,
PG_KRB_SRVTAB,
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -4188,7 +4188,8 @@ static struct config_string ConfigureNamesString[] =
{
/*
* Can't be set by ALTER SYSTEM as it can lead to recursive definition
- * of data_directory.
+ * of data_directory. check_canonical_path() is not needed here as
+ * canonicalization is done when loading configuration files.
*/
{"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's data directory."),
@@ -4201,6 +4202,10 @@ static struct config_string ConfigureNamesString[] =
},
{
+ /*
+ * check_canonical_path() is not needed here as canonicalization
+ * is done when loading the configuration file.
+ */
{"config_file", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's main configuration file."),
NULL,
@@ -4212,6 +4217,10 @@ static struct config_string ConfigureNamesString[] =
},
{
+ /*
+ * check_canonical_path() is not needed here as canonicalization
+ * is done when loading the configuration file.
+ */
{"hba_file", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's \"hba\" configuration file."),
NULL,
@@ -4223,6 +4232,10 @@ static struct config_string ConfigureNamesString[] =
},
{
+ /*
+ * check_canonical_path() is not needed here as canonicalization
+ * is done when loading the configuration file.
+ */
{"ident_file", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's \"ident\" configuration file."),
NULL,
@@ -4266,7 +4279,7 @@ static struct config_string ConfigureNamesString[] =
},
&ssl_cert_file,
"server.crt",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -4276,7 +4289,7 @@ static struct config_string ConfigureNamesString[] =
},
&ssl_key_file,
"server.key",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -4286,7 +4299,7 @@ static struct config_string ConfigureNamesString[] =
},
&ssl_ca_file,
"",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -4296,7 +4309,7 @@ static struct config_string ConfigureNamesString[] =
},
&ssl_crl_file,
"",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
@@ -4369,7 +4382,7 @@ static struct config_string ConfigureNamesString[] =
},
&ssl_dh_params_file,
"",
- NULL, NULL, NULL
+ check_canonical_path, NULL, NULL
},
{
signature.asc
Description: PGP signature
