Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
I wrote:
> Meh. I guess there's not much point in arguing with facts on the
> ground. Anders' proposed behavior seems like the way to go then,
> at least so far as libpq is concerned.
So I pushed that, but while working on it I grew quite annoyed
at the messy API exhibited by src/port/thread.c, particularly
at how declaring its functions in port.h requires #including
and there. That means those headers are
read by every compile in our tree, though only a tiny number
of modules actually need either. So here are a couple of
follow-on patches to improve that situation.
For pqGethostbyname, there is no consumer other than
src/port/getaddrinfo.c, which makes it even sillier because that
file isn't even compiled on a large majority of platforms, making
pqGethostbyname dead code that we nonetheless build everywhere.
So 0001 attached just moves that function into getaddrinfo.c.
For pqGetpwuid, the best solution seemed to be to add a
less platform-dependent API layer, which I did in 0002
attached. Perhaps someone would object to the API details
I chose, but by and large this seems like an improvement
that reduces the amount of code duplication in the callers.
Thoughts?
regards, tom lane
diff --git a/src/include/port.h b/src/include/port.h
index 22ea292a6d..df81fa97c6 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -14,7 +14,6 @@
#define PG_PORT_H
#include
-#include
#include
/*
@@ -485,12 +484,6 @@ extern int pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
size_t buflen, struct passwd **result);
#endif
-extern int pqGethostbyname(const char *name,
- struct hostent *resultbuf,
- char *buffer, size_t buflen,
- struct hostent **result,
- int *herrno);
-
extern void pg_qsort(void *base, size_t nel, size_t elsize,
int (*cmp) (const void *, const void *));
extern int pg_qsort_strcmp(const void *a, const void *b);
diff --git a/src/port/Makefile b/src/port/Makefile
index b3754d8940..bfe1feb0d4 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -84,6 +84,10 @@ libpgport.a: $(OBJS)
rm -f $@
$(AR) $(AROPT) $@ $^
+# getaddrinfo.o and getaddrinfo_shlib.o need PTHREAD_CFLAGS (but getaddrinfo_srv.o does not)
+getaddrinfo.o: CFLAGS+=$(PTHREAD_CFLAGS)
+getaddrinfo_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
+
# thread.o and thread_shlib.o need PTHREAD_CFLAGS (but thread_srv.o does not)
thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c
index b0ca4c778e..3284c6eb52 100644
--- a/src/port/getaddrinfo.c
+++ b/src/port/getaddrinfo.c
@@ -34,6 +34,14 @@
#include "port/pg_bswap.h"
+#ifdef FRONTEND
+static int pqGethostbyname(const char *name,
+ struct hostent *resultbuf,
+ char *buffer, size_t buflen,
+ struct hostent **result,
+ int *herrno);
+#endif
+
#ifdef WIN32
/*
* The native routines may or may not exist on the Windows platform we are on,
@@ -394,3 +402,39 @@ getnameinfo(const struct sockaddr *sa, int salen,
return 0;
}
+
+/*
+ * Wrapper around gethostbyname() or gethostbyname_r() to mimic
+ * POSIX gethostbyname_r() behaviour, if it is not available or required.
+ */
+#ifdef FRONTEND
+static int
+pqGethostbyname(const char *name,
+struct hostent *resultbuf,
+char *buffer, size_t buflen,
+struct hostent **result,
+int *herrno)
+{
+#if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R)
+
+ /*
+ * broken (well early POSIX draft) gethostbyname_r() which returns 'struct
+ * hostent *'
+ */
+ *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno);
+ return (*result == NULL) ? -1 : 0;
+#else
+
+ /* no gethostbyname_r(), just use gethostbyname() */
+ *result = gethostbyname(name);
+
+ if (*result != NULL)
+ *herrno = h_errno;
+
+ if (*result != NULL)
+ return 0;
+ else
+ return -1;
+#endif
+}
+#endif /* FRONTEND */
diff --git a/src/port/thread.c b/src/port/thread.c
index c1040d4e24..a4c1672c89 100644
--- a/src/port/thread.c
+++ b/src/port/thread.c
@@ -76,41 +76,3 @@ pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
#endif
}
#endif
-
-/*
- * Wrapper around gethostbyname() or gethostbyname_r() to mimic
- * POSIX gethostbyname_r() behaviour, if it is not available or required.
- * This function is called _only_ by our getaddrinfo() portability function.
- */
-#ifndef HAVE_GETADDRINFO
-int
-pqGethostbyname(const char *name,
-struct hostent *resultbuf,
-char *buffer, size_t buflen,
-struct hostent **result,
-int *herrno)
-{
-#if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R)
-
- /*
- * broken (well early POSIX draft) gethostbyname_r() which returns 'struct
- * hostent *'
- */
- *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno);
- return (*result == NULL) ? -1 : 0;
-#else
-
- /* no gethostbyname_r(), just use gethostbyname() */
- *result = gethostbyname(name);
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
Christoph Moench-Tegeder writes: > ## Tom Lane (t...@sss.pgh.pa.us): >> Isn't that a flat out violation of POSIX 8.3 Other Environment Variables? > After poking around across some Linuxes, it looks like people silently > agreed that "services" are not logged-in users: among the daemons, > having HOME set (as observed in /proc/*/environ) is an exception, > not the norm. Meh. I guess there's not much point in arguing with facts on the ground. Anders' proposed behavior seems like the way to go then, at least so far as libpq is concerned. (I remain skeptical that psql would be run in such an environment, but there's no value in making it act different from libpq.) regards, tom lane
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
## Tom Lane (t...@sss.pgh.pa.us): > Isn't that a flat out violation of POSIX 8.3 Other Environment Variables? > > HOME > The system shall initialize this variable at the time of login to > be a pathname of the user's home directory. See . > > To claim it's not, you have to claim these programs aren't logged in, > in which case where did they get any privileges from? After poking around across some Linuxes, it looks like people silently agreed that "services" are not logged-in users: among the daemons, having HOME set (as observed in /proc/*/environ) is an exception, not the norm. I'm not sure if that's a "new" thing with systemd, I don't have a linux with pure SysV-init available (but I guess those are rare animals anyways). Regards, Christoph -- Spare Space
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
Christoph Moench-Tegeder writes: > ## Tom Lane (t...@sss.pgh.pa.us): >> Given the POSIX requirements, it's basically impossible to believe >> that there are interesting cases where $HOME isn't set. > When I look at a random Debian with the usual PGDG packages, the > postmaster process (and every backend) has a rather minimal environment > without HOME. When I remember the code correctly, walreceiver uses > the functions from fe-connect.c and may need to find the service file, > a password file or certificates. If I'm correct with that, requiring > HOME to be set would be a significant change for existing "normal" > installations. > What about containers and similar "reduced" environments? Isn't that a flat out violation of POSIX 8.3 Other Environment Variables? HOME The system shall initialize this variable at the time of login to be a pathname of the user's home directory. See . To claim it's not, you have to claim these programs aren't logged in, in which case where did they get any privileges from? regards, tom lane
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
## Tom Lane (t...@sss.pgh.pa.us): > Given the POSIX requirements, it's basically impossible to believe > that there are interesting cases where $HOME isn't set. When I look at a random Debian with the usual PGDG packages, the postmaster process (and every backend) has a rather minimal environment without HOME. When I remember the code correctly, walreceiver uses the functions from fe-connect.c and may need to find the service file, a password file or certificates. If I'm correct with that, requiring HOME to be set would be a significant change for existing "normal" installations. What about containers and similar "reduced" environments? Regards, Christoph -- Spare Space
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On Sun, Jan 09, 2022 at 01:59:02PM -0500, Tom Lane wrote: > Given the POSIX requirements, it's basically impossible to believe > that there are interesting cases where $HOME isn't set. I've run into this before - children of init may not have HOME set. It's easy enough to add it if it's needed, but should probably be called out in the release notes. -- Justin
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On 1/9/22 13:04, Tom Lane wrote: The only case that the v1 patch helps such a user for is if they unset HOME or set it precisely to ''. If they set it to anything else, it's still broken from their perspective. So I do not find that that argument holds water. Moreover, ISTM that the only plausible use-case for unsetting HOME is to prevent programs from finding stuff in your home directory. What would be the point otherwise? So it's pretty hard to envision a case where somebody is actually using, and happy with, the behavior you argue we ought to keep. Obviously a user who intentionally breaks their environment should expect problems. But what I’m saying is that a user could have written a script that unsets HOME by *accident* while intending to clear *other* things out of the environment. They might have developed it by starting with an empty environment and adding back the minimal set of variables they needed to get something to work. Since most programs (including most libcs and shells) do in fact fall back to getpwuid when HOME is unset, they may not have noticed an unset HOME as a problem. Unsetting HOME does not, in practice, prevent most programs from finding stuff in your home directory. Anders
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
Anders Kaseorg writes: > On 1/9/22 10:59, Tom Lane wrote: >> 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. > While POSIX requires that the login program put you in a conforming > environment, nothing stops the user from building a non-conforming > environment, such as with ‘env -i’. One could argue that such a user > deserves whatever broken behavior they might get. But to me it seems > prudent to continue working there if it worked before. The only case that the v1 patch helps such a user for is if they unset HOME or set it precisely to ''. If they set it to anything else, it's still broken from their perspective. So I do not find that that argument holds water. Moreover, ISTM that the only plausible use-case for unsetting HOME is to prevent programs from finding stuff in your home directory. What would be the point otherwise? So it's pretty hard to envision a case where somebody is actually using, and happy with, the behavior you argue we ought to keep. >> The v1 patch also neglects the matter of documentation. > The reason I didn’t change the documentation is that this is already > what “~” is supposed to mean according to POSIX and common > implementations. The point here is precisely that we're changing what *we* think ~ means. I don't think we can just leave the docs unchanged. regards, tom lane
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On 1/9/22 10:59, Tom Lane wrote: 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. While POSIX requires that the login program put you in a conforming environment, nothing stops the user from building a non-conforming environment, such as with ‘env -i’. One could argue that such a user deserves whatever broken behavior they might get. But to me it seems prudent to continue working there if it worked before. 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.) The reason I didn’t change the documentation is that this is already what “~” is supposed to mean according to POSIX and common implementations. See previous discussion: https://www.postgresql.org/message-id/163425265.90107%40mit.edu https://www.postgresql.org/message-id/d452fd57-8c34-0a94-79c1-4498eb4ffbdc%40mit.edu I consider my patch a bug fix that implements the behavior one would already expect from the existing documentation. Therefore, I still prefer my v1 patch on both counts. I am willing to be overruled if you still disagree, but I wanted to explain my reasoning. Anders
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
Anders Kaseorg 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 Specifies the name of the file used to store passwords (see ). - Defaults to ~/.pgpass, or + Defaults to $HOME/.pgpass, or %APPDATA%\postgresql\pgpass.conf on Microsoft Windows. (No error is reported if this file does not exist.) @@ -1670,7 +1670,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname This parameter specifies the file name of the client SSL certificate, replacing the default -~/.postgresql/postgresql.crt. +$HOME/.postgresql/postgresql.crt. This parameter is ignored if an SSL connection is not made. @@ -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 -~/.postgresql/postgresql.key, or it can specify a key +$HOME/.postgresql/postgresql.key, or it can specify a key obtained from an external engine (engines are OpenSSL 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 (CA) certificate(s). If the file exists, the server's certificate will be verified to be signed by one of these authorities. The default is -~/.postgresql/root.crt. +$HOME/.postgresql/root.crt. @@ -1749,7 +1749,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname nor is set, this setting is taken as -~/.postgresql/root.crl. +$HOME/.postgresql/root.crl. @@ -7776,7 +7776,7 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) PGSERVICEFILE specifies the name of the per-user connection service file. If not set, it defaults - to ~/.pg_service.conf + to $HOME/.pg_service.conf (see ). @@ -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 ~/.pg_service.conf; this can be overridden by + at $HOME/.pg_service.conf; this can be overridden by setting the environment variable PGSERVICEFILE. The system-wide file is named pg_service.conf. By default it is sought in the etc directory @@ -8354,8 +8354,8 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*) To allow server certificate verification, one or more root certificates - must be placed in the file ~/.postgresql/root.crt - in the user's home directory. (On Microsoft Windows the file is named + must be placed in the file $HOME/.postgresql/root.crt. + (On Microsoft Windows the file is named
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
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. POSIX specifies that the shell uses the HOME environment variable for ‘cd’ with no arguments and for the expansion of ~. This implies by reference that this behavior is required of wordexp() as well. https://pubs.opengroup.org/onlinepubs/9699919799/utilities/cd.html https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_01 https://pubs.opengroup.org/onlinepubs/9699919799/functions/wordexp.html libc’s glob() and wordexp() respect HOME in glibc, musl, NetBSD, and FreeBSD. https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/glob.c;hb=glibc-2.34#l622 https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/wordexp.c;hb=glibc-2.34#l293 https://git.musl-libc.org/cgit/musl/tree/src/regex/glob.c?h=v1.2.2#n203 https://git.musl-libc.org/cgit/musl/tree/src/misc/wordexp.c?h=v1.2.2#n111 https://github.com/NetBSD/src/blob/netbsd-9/lib/libc/gen/glob.c#L424 https://github.com/NetBSD/src/blob/netbsd-9/lib/libc/gen/wordexp.c#L129-L150 https://github.com/NetBSD/src/blob/netbsd-9/bin/sh/expand.c#L434-L441 https://github.com/freebsd/freebsd-src/blob/release/13.0.0/lib/libc/gen/glob.c#L457 https://github.com/freebsd/freebsd-src/blob/release/13.0.0/lib/libc/gen/wordexp.c#L171-L190 https://github.com/freebsd/freebsd-src/blob/release/13.0.0/bin/sh/expand.c#L396 (Today I learned that musl and BSD libc literally spawn a shell process to handle wordexp(). Wow.) Anders
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
> On 20 Oct 2021, at 07:40, Kyotaro Horiguchi wrote: > > At Tue, 19 Oct 2021 02:44:03 -0700, Anders Kaseorg wrote in >> On 10/19/21 01:34, Kyotaro Horiguchi wrote: >>> I tend to agree to this, but seeing ssh ignoring $HOME, I'm not sure >>> it's safe that we follow the variable at least when accessing >>> confidentiality(?) files. Since I don't understand the exact >>> reasoning for the ssh's behavior so it's just my humbole opinion. >> >> According to https://bugzilla.mindrot.org/show_bug.cgi?id=3048#c1, it >> used to be supported to install the ssh binary as setuid. A >> setuid/setgid binary needs to treat all environment variables with >> suspicion: if it can be convinced to write a file to $HOME with root >> privileges, then a user who modifies $HOME before invoking the binary >> could cause it to write to a file that the user normally couldn’t. >> >> There’s no such concern for a binary that isn’t setuid/setgid. Anyone >> with the ability to modify $HOME can be assumed to already have full >> control of the user account. > > Thansk for the link. Still I'm not sure it's the fact but it sounds > reasonable enough. If that's the case, I vote +1 for psql or other > commands honoring $HOME. 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. -- Daniel Gustafsson https://vmware.com/
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
At Tue, 19 Oct 2021 02:44:03 -0700, Anders Kaseorg wrote in > On 10/19/21 01:34, Kyotaro Horiguchi wrote: > > I tend to agree to this, but seeing ssh ignoring $HOME, I'm not sure > > it's safe that we follow the variable at least when accessing > > confidentiality(?) files. Since I don't understand the exact > > reasoning for the ssh's behavior so it's just my humbole opinion. > > According to https://bugzilla.mindrot.org/show_bug.cgi?id=3048#c1, it > used to be supported to install the ssh binary as setuid. A > setuid/setgid binary needs to treat all environment variables with > suspicion: if it can be convinced to write a file to $HOME with root > privileges, then a user who modifies $HOME before invoking the binary > could cause it to write to a file that the user normally couldn’t. > > There’s no such concern for a binary that isn’t setuid/setgid. Anyone > with the ability to modify $HOME can be assumed to already have full > control of the user account. Thansk for the link. Still I'm not sure it's the fact but it sounds reasonable enough. If that's the case, I vote +1 for psql or other commands honoring $HOME. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On 10/19/21 01:34, Kyotaro Horiguchi wrote: I tend to agree to this, but seeing ssh ignoring $HOME, I'm not sure it's safe that we follow the variable at least when accessing confidentiality(?) files. Since I don't understand the exact reasoning for the ssh's behavior so it's just my humbole opinion. According to https://bugzilla.mindrot.org/show_bug.cgi?id=3048#c1, it used to be supported to install the ssh binary as setuid. A setuid/setgid binary needs to treat all environment variables with suspicion: if it can be convinced to write a file to $HOME with root privileges, then a user who modifies $HOME before invoking the binary could cause it to write to a file that the user normally couldn’t. There’s no such concern for a binary that isn’t setuid/setgid. Anyone with the ability to modify $HOME can be assumed to already have full control of the user account. Anders
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
At Mon, 18 Oct 2021 19:23:50 -0300, Alvaro Herrera
wrote in
> On 2021-Oct-14, Anders Kaseorg wrote:
>
> > This is important for systems where many users share the same UID, and
> > for test systems that change HOME to avoid interference with the
> > user’s real home directory. It matches what most applications do, as
> > well as what glibc does for glob("~", GLOB_TILDE, …) and wordexp("~",
> > …).
> >
> > There was some previous discussion of this in 2016, where although
> > there were some questions about the use case, there seemed to be
> > general support for the concept:
> >
> > https://www.postgresql.org/message-id/flat/CAEH6cQqbdbXoUHJBbX9ixwfjFFsUC-a8hFntKcci%3DdiWgBb3fQ%40mail.gmail.com
>
> I think modifying $HOME is a strange way to customize things, but given
> how widespread it is [claimed to be] today, it seems reasonable to do
> things that way.
I tend to agree to this, but seeing ssh ignoring $HOME, I'm not sure
it's safe that we follow the variable at least when accessing
confidentiality(?) files. Since I don't understand the exact
reasoning for the ssh's behavior so it's just my humbole opinion.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On Mon, Oct 18, 2021 at 07:23:50PM -0300, Alvaro Herrera wrote: > I think modifying $HOME is a strange way to customize things, but given > how widespread it is [claimed to be] today, it seems reasonable to do > things that way. I am not sure about this claim, but it seems to me that we could get rid of the duplications in src/port/path.c, libpq/fe-connect.c and psql/command.c (this one is different for WIN32 but consistency would be a good thing) as the proposed patch outlines. So I would suggest to begin with that rather than changing three places to do the same thing. -- Michael signature.asc Description: PGP signature
Re: [PATCH] Prefer getenv("HOME") to find the UNIX home directory
On 2021-Oct-14, Anders Kaseorg wrote:
> This is important for systems where many users share the same UID, and
> for test systems that change HOME to avoid interference with the
> user’s real home directory. It matches what most applications do, as
> well as what glibc does for glob("~", GLOB_TILDE, …) and wordexp("~",
> …).
>
> There was some previous discussion of this in 2016, where although
> there were some questions about the use case, there seemed to be
> general support for the concept:
>
> https://www.postgresql.org/message-id/flat/CAEH6cQqbdbXoUHJBbX9ixwfjFFsUC-a8hFntKcci%3DdiWgBb3fQ%40mail.gmail.com
I think modifying $HOME is a strange way to customize things, but given
how widespread it is [claimed to be] today, it seems reasonable to do
things that way.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
[PATCH] Prefer getenv("HOME") to find the UNIX home directory
According to getpwnam(3):
An application that wants to determine its user's home directory
should inspect the value of HOME (rather than the value
getpwuid(getuid())->pw_dir) since this allows the user to modify
their notion of "the home directory" during a login session.
This is important for systems where many users share the same UID, and for test
systems that change HOME to avoid interference with the user’s real home
directory. It matches what most applications do, as well as what glibc does
for glob("~", GLOB_TILDE, …) and wordexp("~", …).
There was some previous discussion of this in 2016, where although there were
some questions about the use case, there seemed to be general support for the
concept:
https://www.postgresql.org/message-id/flat/CAEH6cQqbdbXoUHJBbX9ixwfjFFsUC-a8hFntKcci%3DdiWgBb3fQ%40mail.gmail.com
Regardless of whether one thinks modifying HOME is a good idea, if we happen to
find ourselves in that case, we should respect the modified HOME, so that when
the user creates (say) a ~/.pgpass file, we’ll look for it at the same place
the user’s editor created it. getenv() also skips the overhead of reading
/etc/passwd as an added bonus.
The way I ran into this issue myself was in a test suite that runs on GitHub
Actions, which automatically sets HOME=/github/home.
Anders
From df9c435c759fffe77c9c92f70e7c095ffb6556ae Mon Sep 17 00:00:00 2001
From: Anders Kaseorg
Date: Thu, 14 Oct 2021 15:20:13 -0700
Subject: [PATCH v1] Prefer getenv("HOME") to find the UNIX home directory
According to getpwnam(3):
An application that wants to determine its user's home directory
should inspect the value of HOME (rather than the value
getpwuid(getuid())->pw_dir) since this allows the user to modify
their notion of "the home directory" during a login session.
Signed-off-by: Anders Kaseorg
---
src/bin/psql/command.c| 20
src/interfaces/libpq/fe-connect.c | 14 ++
src/port/path.c | 14 ++
3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..cc2fe6ba0e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -15,6 +15,7 @@
#include /* for stat() */
#include /* for setitimer() */
#include /* open() flags */
+#include /* for getenv() */
#include /* for geteuid(), getpid(), stat() */
#else
#include
@@ -558,15 +559,18 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd)
uid_t user_id = geteuid();
errno = 0; /* clear errno before call */
- pw = getpwuid(user_id);
- if (!pw)
- {
-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);
+ dir = getenv("HOME");
+ if (dir == NULL || dir[0] == '\0') {
+pw = getpwuid(user_id);
+if (!pw)
+{
+ 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);
+}
+dir = pw->pw_dir;
}
- dir = pw->pw_dir;
#else /* WIN32 */
/*
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index b288d346f9..ebdd815c73 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include "common/ip.h"
@@ -7242,14 +7243,19 @@ bool
pqGetHomeDirectory(char *buf, int bufsize)
{
#ifndef WIN32
+ const char *home;
char pwdbuf[BUFSIZ];
struct passwd pwdstr;
struct passwd *pwd = NULL;
- (void) pqGetpwuid(geteuid(), , pwdbuf, sizeof(pwdbuf), );
- if (pwd == NULL)
- return false;
- strlcpy(buf, pwd->pw_dir, bufsize);
+ home = getenv("HOME");
+ if (home == NULL || home[0] == '\0') {
+ (void) pqGetpwuid(geteuid(), , pwdbuf, sizeof(pwdbuf), );
+ if (pwd == NULL)
+ return false;
+ home = pwd->pw_dir;
+ }
+ strlcpy(buf, home, bufsize);
return true;
#else
char tmppath[MAX_PATH];
diff --git a/src/port/path.c b/src/port/path.c
index c39d4688cd..607bd16c23 100644
--- a/src/port/path.c
+++ b/src/port/path.c
@@ -32,6 +32,7 @@
#define near
#include
#else
+#include
#include
#endif
@@ -807,14 +808,19 @@ bool
get_home_path(char *ret_path)
{
#ifndef WIN32
+ const char *home;
char pwdbuf[BUFSIZ];
struct passwd pwdstr;
struct passwd *pwd = NULL;
- (void) pqGetpwuid(geteuid(), , pwdbuf, sizeof(pwdbuf), );
- if (pwd == NULL)
- return false;
- strlcpy(ret_path, pwd->pw_dir, MAXPGPATH);
+ home = getenv("HOME");
+ if (home == NULL || home[0] == '\0') {
+ (void) pqGetpwuid(geteuid(), , pwdbuf, sizeof(pwdbuf), );
+ if (pwd == NULL)
+ return false;
+ home = pwd->pw_dir;
+ }
+ strlcpy(ret_path, home, MAXPGPATH);
return true;
#else
char *tmppath;
--
2.33.0
