Re: Expand the use of check_canonical_path() for more GUCs

> On May 20, 2020, at 12:03 AM, Michael Paquier <michael@paquier.xyz> wrote:
>
> 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?

Before this patch, there are three GUCs that get check_canonical_path treatment:

   log_directory
   external_pid_file
   stats_temp_directory

After the patch, these also get the treatment (though Peter seems to be objecting to the change):

   promote_trigger_file
   krb_server_keyfile
   ssl_cert_file
   ssl_key_file
   ssl_ca_file
   ssl_crl_file
   ssl_dh_params_file

and these still don't, with comments about how they are already canonicalized when the config file is loaded:

   data_directory
   config_file
   hba_file
   ident_file

A little poking around shows that in SelectConfigFiles(), these four directories were set by SetConfigOption().  I
don'tsee a problem with the code, but the way this stuff is spread around makes it easy for somebody adding a new GUC
filepath to do it wrong.  I don't have much opinion about Peter's preference that paths be left alone, but I'd prefer
somecomments in guc.c explaining it all.  The only cleanup that occurs to me is to reorder ConfigureNamesString[] to
haveall the path options back-to-back, with the four that are set by SelectConfigFiles() at the top with comments about
whythey are special, and the rest after that with comments about why they need or do not need canonicalization. 


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company