Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory - Mailing list pgsql-hackers

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

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 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.sgml
index 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.c
index 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.c
index 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.c
index 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;