From: Tom Lane <tgl@sss.pgh.pa.us>
To: Anders Kaseorg <andersk@mit.edu>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: alvherre@alvh.no-ip.org
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
Date: Sun, 09 Jan 2022 13:59:02 -0500
Message-ID: <1769427.1641754742@sss.pgh.pa.us> (raw)
In-Reply-To: <d452fd57-8c34-0a94-79c1-4498eb4ffbdc@mit.edu>
References: <202110182223.zeze33l55qa5@alvherre.pgsql>
<20211019.173422.706584612464055211.horikyota.ntt@gmail.com>
<5d675cbe-7837-e9ae-961d-be7d03201f3b@mit.edu>
<20211020.144014.580443822870543857.horikyota.ntt@gmail.com>
<07CA67A0-A46C-4ED5-99C6-24D41BDDC8CA@yesql.se>
<d452fd57-8c34-0a94-79c1-4498eb4ffbdc@mit.edu>
Anders Kaseorg <andersk@mit.edu> writes:
> On 10/20/21 04:55, Daniel Gustafsson wrote:
>> Is the proposed change portable across all linux/unix systems we support?
>> Reading aobut indicates that it's likely to be, but neither NetBSD nor FreeBSD
>> have the upthread referenced wording in their manpages.
> Since the proposed change falls back to the old behavior if HOME is
> unset or empty, I assume this is a question about convention and not
> literally about whether it will work on these systems. I don’t find it
> surprising that this convention isn’t explicitly called out in every
> system’s manpage for the wrong function, but it still applies to these
> systems.
Given the POSIX requirements, it's basically impossible to believe
that there are interesting cases where $HOME isn't set. Thus, it
seems to me that keeping the getpwuid calls will just mean carrying
untestable dead code, so we should simplify matters by ripping
those out and *only* consulting $HOME.
The v1 patch also neglects the matter of documentation. I think
the simplest and most transparent thing to do is just to explicitly
mention $HOME everyplace we talk about files that are sought there,
in place of our current convention to write "~". (I'm too lazy
to go digging in the git history, but I have a feeling that this is
undoing somebody's intentional change from a long time back.)
BTW, not directly impacted by this patch but adjacent to it,
I noted that on Windows psql's \cd defaults to changing to "/".
That seems a bit surprising, and we definitely fail to document it.
I settled for noting it in the documentation, but should we make
it do something else?
PFA v2 patch.
regards, tom lane
Attachments:v2-0001-use-HOME-for-home-directory.patch (text/x-diff, 13.3K) downloaddiff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgmlindex 14f35d37f6..faf36f051f 100644--- a/doc/src/sgml/libpq.sgml+++ b/doc/src/sgml/libpq.sgml@@ -1214,7 +1214,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
<para>
Specifies the name of the file used to store passwords
(see <xref linkend="libpq-pgpass"/>).
- Defaults to <filename>~/.pgpass</filename>, or+ Defaults to <filename>$HOME/.pgpass</filename>, or
<filename>%APPDATA%\postgresql\pgpass.conf</filename> on Microsoft Windows.
(No error is reported if this file does not exist.)
</para>
@@ -1670,7 +1670,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
<para>
This parameter specifies the file name of the client SSL
certificate, replacing the default
- <filename>~/.postgresql/postgresql.crt</filename>.+ <filename>$HOME/.postgresql/postgresql.crt</filename>.
This parameter is ignored if an SSL connection is not made.
</para>
</listitem>
@@ -1683,7 +1683,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
This parameter specifies the location for the secret key used for
the client certificate. It can either specify a file name that will
be used instead of the default
- <filename>~/.postgresql/postgresql.key</filename>, or it can specify a key+ <filename>$HOME/.postgresql/postgresql.key</filename>, or it can specify a key
obtained from an external <quote>engine</quote> (engines are
<productname>OpenSSL</productname> loadable modules). An external engine
specification should consist of a colon-separated engine name and
@@ -1733,7 +1733,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
certificate authority (<acronym>CA</acronym>) certificate(s).
If the file exists, the server's certificate will be verified
to be signed by one of these authorities. The default is
- <filename>~/.postgresql/root.crt</filename>.+ <filename>$HOME/.postgresql/root.crt</filename>.
</para>
</listitem>
</varlistentry>
@@ -1749,7 +1749,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
<xref linkend='libpq-connect-sslcrl'/> nor
<xref linkend='libpq-connect-sslcrldir'/> is set, this setting is
taken as
- <filename>~/.postgresql/root.crl</filename>.+ <filename>$HOME/.postgresql/root.crl</filename>.
</para>
</listitem>
</varlistentry>
@@ -7776,7 +7776,7 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</indexterm>
<envar>PGSERVICEFILE</envar> specifies the name of the per-user
connection service file. If not set, it defaults
- to <filename>~/.pg_service.conf</filename>+ to <filename>$HOME/.pg_service.conf</filename>
(see <xref linkend="libpq-pgservice"/>).
</para>
</listitem>
@@ -8151,7 +8151,7 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
system-wide file. If the same service name exists in both the user
and the system file, the user file takes precedence.
By default, the per-user service file is located
- at <filename>~/.pg_service.conf</filename>; this can be overridden by+ at <filename>$HOME/.pg_service.conf</filename>; this can be overridden by
setting the environment variable <envar>PGSERVICEFILE</envar>.
The system-wide file is named <filename>pg_service.conf</filename>.
By default it is sought in the <filename>etc</filename> directory
@@ -8354,8 +8354,8 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
<para>
To allow server certificate verification, one or more root certificates
- must be placed in the file <filename>~/.postgresql/root.crt</filename>- in the user's home directory. (On Microsoft Windows the file is named+ must be placed in the file <filename>$HOME/.postgresql/root.crt</filename>.+ (On Microsoft Windows the file is named
<filename>%APPDATA%\postgresql\root.crt</filename>.) Intermediate
certificates should also be added to the file if they are needed to link
the certificate chain sent by the server to the root certificates
@@ -8364,7 +8364,7 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
<para>
Certificate Revocation List (CRL) entries are also checked
- if the file <filename>~/.postgresql/root.crl</filename> exists+ if the file <filename>$HOME/.postgresql/root.crl</filename> exists
(<filename>%APPDATA%\postgresql\root.crl</filename> on Microsoft
Windows).
</para>
@@ -8398,10 +8398,10 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
If the server attempts to verify the identity of the
client by requesting the client's leaf certificate,
<application>libpq</application> will send the certificates stored in
- file <filename>~/.postgresql/postgresql.crt</filename> in the user's home- directory. The certificates must chain to the root certificate trusted+ the file <filename>$HOME/.postgresql/postgresql.crt</filename>.+ The certificates must chain to the root certificate trusted
by the server. A matching
- private key file <filename>~/.postgresql/postgresql.key</filename> must also+ private key file <filename>$HOME/.postgresql/postgresql.key</filename> must also
be present. The private
key file must not allow any access to world or group; achieve this by the
command <command>chmod 0600 ~/.postgresql/postgresql.key</command>.
@@ -8643,27 +8643,27 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
<tbody>
<row>
- <entry><filename>~/.postgresql/postgresql.crt</filename></entry>+ <entry><filename>$HOME/.postgresql/postgresql.crt</filename></entry>
<entry>client certificate</entry>
<entry>sent to server</entry>
</row>
<row>
- <entry><filename>~/.postgresql/postgresql.key</filename></entry>+ <entry><filename>$HOME/.postgresql/postgresql.key</filename></entry>
<entry>client private key</entry>
<entry>proves client certificate sent by owner; does not indicate
certificate owner is trustworthy</entry>
</row>
<row>
- <entry><filename>~/.postgresql/root.crt</filename></entry>+ <entry><filename>$HOME/.postgresql/root.crt</filename></entry>
<entry>trusted certificate authorities</entry>
<entry>checks that server certificate is signed by a trusted certificate
authority</entry>
</row>
<row>
- <entry><filename>~/.postgresql/root.crl</filename></entry>+ <entry><filename>$HOME/.postgresql/root.crl</filename></entry>
<entry>certificates revoked by certificate authorities</entry>
<entry>server certificate must not be on this list</entry>
</row>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgmlindex 1ab200a4ad..6b6f5b1864 100644--- a/doc/src/sgml/ref/psql-ref.sgml+++ b/doc/src/sgml/ref/psql-ref.sgml@@ -553,7 +553,7 @@ EOF
<para>
Do not read the start-up file (neither the system-wide
<filename>psqlrc</filename> file nor the user's
- <filename>~/.psqlrc</filename> file).+ <filename>$HOME/.psqlrc</filename> file).
</para>
</listitem>
</varlistentry>
@@ -677,7 +677,7 @@ EOF
<envar>PGPORT</envar> and/or <envar>PGUSER</envar> to appropriate
values. (For additional environment variables, see <xref
linkend="libpq-envars"/>.) It is also convenient to have a
- <filename>~/.pgpass</filename> file to avoid regularly having to type in+ <filename>$HOME/.pgpass</filename> file to avoid regularly having to type in
passwords. See <xref linkend="libpq-pgpass"/> for more information.
</para>
@@ -984,7 +984,8 @@ testdb=>
<para>
Changes the current working directory to
<replaceable>directory</replaceable>. Without argument, changes
- to the current user's home directory.+ to the current user's home directory (<literal>$HOME</literal>), or+ the root directory on Windows.
</para>
<tip>
@@ -3791,7 +3792,7 @@ bar
behavior, but autocommit-off is closer to the SQL spec. If you
prefer autocommit-off, you might wish to set it in the system-wide
<filename>psqlrc</filename> file or your
- <filename>~/.psqlrc</filename> file.+ <filename>$HOME/.psqlrc</filename> file.
</para>
</note>
</listitem>
@@ -3960,13 +3961,13 @@ bar
The file name that will be used to store the history list. If unset,
the file name is taken from the <envar>PSQL_HISTORY</envar>
environment variable. If that is not set either, the default
- is <filename>~/.psql_history</filename>,+ is <filename>$HOME/.psql_history</filename>,
or <filename>%APPDATA%\postgresql\psql_history</filename> on Windows.
For example, putting:
<programlisting>
\set HISTFILE ~/.psql_history-:DBNAME
</programlisting>
- in <filename>~/.psqlrc</filename> will cause+ in <filename>$HOME/.psqlrc</filename> will cause
<application>psql</application> to maintain a separate history for
each database.
</para>
@@ -4774,13 +4775,13 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
<variablelist>
<varlistentry>
- <term><filename>psqlrc</filename> and <filename>~/.psqlrc</filename></term>+ <term><filename>psqlrc</filename> and <filename>$HOME/.psqlrc</filename></term>
<listitem>
<para>
Unless it is passed an <option>-X</option> option,
<application>psql</application> attempts to read and execute commands
from the system-wide startup file (<filename>psqlrc</filename>) and then
- the user's personal startup file (<filename>~/.psqlrc</filename>), after+ the user's personal startup file (<filename>$HOME/.psqlrc</filename>), after
connecting to the database but before accepting normal commands.
These files can be used to set up the client and/or the server to taste,
typically with <command>\set</command> and <command>SET</command>
@@ -4809,8 +4810,8 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
can be made <application>psql</application>-version-specific
by appending a dash and the <productname>PostgreSQL</productname>
major or minor release number to the file name,
- for example <filename>~/.psqlrc-9.2</filename> or- <filename>~/.psqlrc-9.2.5</filename>. The most specific+ for example <filename>$HOME/.psqlrc-9.2</filename> or+ <filename>$HOME/.psqlrc-9.2.5</filename>. The most specific
version-matching file will be read in preference to a
non-version-specific file.
</para>
@@ -4822,7 +4823,7 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
<listitem>
<para>
The command-line history is stored in the file
- <filename>~/.psql_history</filename>, or+ <filename>$HOME/.psql_history</filename>, or
<filename>%APPDATA%\postgresql\psql_history</filename> on Windows.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.cindex 053332e7af..e2ea0d11aa 100644--- a/src/bin/psql/command.c+++ b/src/bin/psql/command.c@@ -558,19 +558,13 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd)
else
{
#ifndef WIN32
- struct passwd *pw;- uid_t user_id = geteuid();-- errno = 0; /* clear errno before call */- pw = getpwuid(user_id);- if (!pw)+ /* On Unix-like systems, consult $HOME */+ dir = getenv("HOME");+ if (dir == NULL || dir[0] == '\0')
{
- pg_log_error("could not get home directory for user ID %ld: %s",- (long) user_id,- errno ? strerror(errno) : _("user does not exist"));- exit(EXIT_FAILURE);+ pg_log_error("HOME environment variable is not set");+ success = false;
}
- dir = pw->pw_dir;
#else /* WIN32 */
/*
@@ -581,7 +575,8 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd)
#endif /* WIN32 */
}
- if (chdir(dir) == -1)+ if (success &&+ chdir(dir) < 0)
{
pg_log_error("\\%s: could not change directory to \"%s\": %m",
cmd, dir);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.cindex 72914116ee..f018bf6a9c 100644--- a/src/interfaces/libpq/fe-connect.c+++ b/src/interfaces/libpq/fe-connect.c@@ -7267,14 +7267,11 @@ bool
pqGetHomeDirectory(char *buf, int bufsize)
{
#ifndef WIN32
- char pwdbuf[BUFSIZ];- struct passwd pwdstr;- struct passwd *pwd = NULL;+ const char *home = getenv("HOME");- (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd);- if (pwd == NULL)+ if (home == NULL || home[0] == '\0')
return false;
- strlcpy(buf, pwd->pw_dir, bufsize);+ strlcpy(buf, home, bufsize);
return true;
#else
char tmppath[MAX_PATH];
diff --git a/src/port/path.c b/src/port/path.cindex ee4227ec98..a5a2e9238c 100644--- a/src/port/path.c+++ b/src/port/path.c@@ -807,14 +807,11 @@ bool
get_home_path(char *ret_path)
{
#ifndef WIN32
- char pwdbuf[BUFSIZ];- struct passwd pwdstr;- struct passwd *pwd = NULL;+ const char *home = getenv("HOME");- (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd);- if (pwd == NULL)+ if (home == NULL || home[0] == '\0')
return false;
- strlcpy(ret_path, pwd->pw_dir, MAXPGPATH);+ strlcpy(ret_path, home, MAXPGPATH);
return true;
#else
char *tmppath;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andersk@mit.edu, daniel@yesql.se, horikyota.ntt@gmail.com, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
In-Reply-To: <1769427.1641754742@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox