8000 WIP: Always use Unix-domain sockets in pg_regress on Windows. · Giperboloid/postgres@388719a · GitHub
[go: up one dir, main page]
Skip to content

Commit 388719a

Browse files
macdicemacdice
committed
WIP: Always use Unix-domain sockets in pg_regress on Windows.
Since we can now rely on Unix-domain sockets working on supported Windows versions (10+), we can remove a source of instability and a difference between Unix and Windows in pg_regress. Previously, we thought the socket cleanup code was unsafe, so we made Unix-domain sockets an option with a "use-at-your-own-risk" note. On closer inspection, the concerns about signal handlers don't seem to apply here. (initdb.c has similar concerns but needs separate investigation.) Previously, commit f6dc6dd secured temporary installations using TCP/IP on Windows, while commit be76a6d used file system permissions for Unix sockets on Unix. Now that our src/port/mkdtemp.c file creates non-world-accessible directories on Windows, we can just do the same on Windows.
1 parent 62b1cdb commit 388719a

File tree

1 file changed

+32
-242
lines changed

1 file changed

+32
-242
lines changed

‎src/test/regress/pg_regress.c

Lines changed: 32 additions & 242 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,20 @@ char *host_platform = HOST_TUPLE;
5555
static char *shellprog = SHELLPROG;
5656
#endif
5757

58+
/*
59+
* The name of the environment variable that controls where we put temporary
60+
* files, to override the defaut of "/tmp".
61+
*/
62+
#ifdef WIN32
63+
#define TMPDIR "TMP"
64+
#else
65+
#define TMPDIR "TMPDIR"
66+
#endif
67+
68+
#if defined(WIN32) && defined(USE_ASSERT_CHECKING)
69+
static DWORD main_thread_id;
70+
#endif
71+
5872
/*
5973
* On Windows we use -w in diff switches to avoid problems with inconsistent
6074
* newline representation. The actual result files will generally have
@@ -286,9 +300,7 @@ stop_postmaster(void)
286300
* postmaster exit, so it is indeterminate whether the postmaster has yet to
287301
* unlink the socket and lock file. Unlink them here so we can proceed to
288302
* remove the directory. Ignore errors; leaking a temporary directory is
289-
* unimportant. This can run from a signal handler. The code is not
290-
* acceptable in a Windows signal handler (see initdb.c:trapsig()), but
291-
* on Windows, pg_regress does not use Unix sockets by default.
303+
* unimportant. This can run from a signal handler.
292304
*/
293305
static void
294306
remove_temp(void)
@@ -305,6 +317,18 @@ remove_temp(void)
305317
static void
306318
signal_remove_temp(SIGNAL_ARGS)
307319
{
320+
#ifdef WIN32
321+
/*
322+
* In general, it would not be acceptable to call remove_temp() in a
323+
* Windows signal handler. It is safe in this program though, because
324+
* SIGHUP and SIGPIPE don't really exist and SIGTERM is never raised by the
325+
* system, leaving just SIGINT. SIGINT doesn't interrupt the main
326+
* execution context on Windows, it runs the handler concurrently in
327+
* another thread.
328+
*/
329+
Assert(GetCurrentThreadId() != main_thread_id);
330+
#endif
331+
308332
remove_temp();
309333

310334
pqsignal(postgres_signal_arg, SIG_DFL);
@@ -327,7 +351,7 @@ static const char *
327351
make_temp_sockdir(void)
328352
{
329353
char *template = psprintf("%s/pg_regress-XXXXXX",
330-
getenv("TMPDIR") ? getenv("TMPDIR") : "/tmp");
354+
getenv(TMPDIR) ? getenv(TMPDIR) : "/tmp");
331355

332356
temp_sockdir = mkdtemp(template);
333357
if (temp_sockdir == NULL)
@@ -344,6 +368,10 @@ make_temp_sockdir(void)
344368
/* Remove the directory during clean exit. */
345369
atexit(remove_temp);
346370

371+
#if defined(WIN32) && defined(USE_ASSERT_CHECKING)
372+
main_thread_id = GetCurrentThreadId();
373+
#endif
374+
347375
/*
348376
* Remove the directory before dying to the usual signals. Omit SIGQUIT,
349377
* preserving it as a quick, untidy exit.
@@ -754,211 +782,6 @@ initialize_environment(void)
754782
load_resultmap();
755783
}
756784

757-
#ifdef ENABLE_SSPI
758-
759-
/* support for config_sspi_auth() */
760-
static const char *
761-
fmtHba(const char *raw)
762-
{
763-
static char *ret;
764-
const char *rp;
765-
char *wp;
766-
767-
wp = ret = pg_realloc(ret, 3 + strlen(raw) * 2);
768-
769-
*wp++ = '"';
770-
for (rp = raw; *rp; rp++)
771-
{
772-
if (*rp == '"')
773-
*wp++ = '"';
774-
*wp++ = *rp;
775-
}
776-
*wp++ = '"';
777-
*wp++ = '\0';
778-
779-
return ret;
780-
}
781-
782-
/*
783-
* Get account and domain/realm names for the current user. This is based on
784-
* pg_SSPI_recvauth(). The returned strings use static storage.
785-
*/
786-
static void
787-
current_windows_user(const char **acct, const char **dom)
788-
{
789-
static char accountname[MAXPGPATH];
790-
static char domainname[MAXPGPATH];
791-
HANDLE token;
792-
TOKEN_USER *tokenuser;
793-
DWORD retlen;
794-
DWORD accountnamesize = sizeof(accountname);
795-
DWORD domainnamesize = sizeof(domainname);
796-
SID_NAME_USE accountnameuse;
797-
798-
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &token))
799-
{
800-
fprintf(stderr,
801-
_("%s: could not open process token: error code %lu\n"),
802-
progname, GetLastError());
803-
exit(2);
804-
}
805-
806-
if (!GetTokenInformation(token, TokenUser, NULL, 0, &retlen) && GetLastError() != 122)
807-
{
808-
fprintf(stderr,
809-
_("%s: could not get token information buffer size: error code %lu\n"),
810-
progname, GetLastError());
811-
exit(2);
812-
}
813-
tokenuser = pg_malloc(retlen);
814-
if (!GetTokenInformation(token, TokenUser, tokenuser, retlen, &retlen))
815-
{
816-
fprintf(stderr,
817-
_("%s: could not get token information: error code %lu\n"),
818-
progname, GetLastError());
819-
exit(2);
820-
}
821-
822-
if (!LookupAccountSid(NULL, tokenuser->User.Sid, accountname, &accountnamesize,
823-
domainname, &domainnamesize, &accountnameuse))
824-
{
825-
fprintf(stderr,
826-
_("%s: could not look up account SID: error code %lu\n"),
827-
progname, GetLastError());
828-
exit(2);
829-
}
830-
831-
free(tokenuser);
832-
833-
*acct = accountname;
834-
*dom = domainname;
835-
}
836-
837-
/*
838-
* Rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication. Permit
839-
* the current OS user to authenticate as the bootstrap superuser and as any
840-
* user named in a --create-role option.
841-
*
842-
* In --config-auth mode, the --user switch can be used to specify the
843-
* bootstrap superuser's name, otherwise we assume it is the default.
844-
*/
845-
static void
846-
config_sspi_auth(const char *pgdata, const char *superuser_name)
847-
{
848-
const char *accountname,
849-
*domainname;
850-
char *errstr;
851-
bool have_ipv6;
852-
char fname[MAXPGPATH];
853-
int res;
854-
FILE *hba,
855-
*ident;
856-
_stringlist *sl;
857-
858-
/* Find out the name of the current OS user */
859-
current_windows_user(&accountname, &domainname);
860-
861-
/* Determine the bootstrap superuser's name */
862-
if (superuser_name == NULL)
863-
{
864-
/*
865-
* Compute the default superuser name the same way initdb does.
866-
*
867-
* It's possible that this result always matches "accountname", the
868-
* value SSPI authentication discovers. But the underlying system
869-
* functions do not clearly guarantee that.
870-
*/
871-
superuser_name = get_user_name(&errstr);
872-
if (superuser_name == NULL)
873-
{
874-
fprintf(stderr, "%s: %s\n", progname, errstr);
875-
exit(2);
876-
}
877-
}
878-
879-
/*
880-
* Like initdb.c:setup_config(), determine whether the platform recognizes
881-
* ::1 (IPv6 loopback) as a numeric host address string.
882-
*/
883-
{
884-
struct addrinfo *gai_result;
885-
struct addrinfo hints;
886-
WSADATA wsaData;
887-
888-
hints.ai_flags = AI_NUMERICHOST;
889-
hints.ai_family = AF_UNSPEC;
890-
hints.ai_socktype = 0;
891-
hints.ai_protocol = 0;
892-
hints.ai_addrlen = 0;
893-
hints.ai_canonname = NULL;
894-
hints.ai_addr = NULL;
895-
hints.ai_next = NULL;
896-
897-
have_ipv6 = (WSAStartup(MAKEWORD(2, 2), &wsaData) == 0 &&
898-
getaddrinfo("::1", NULL, &hints, &gai_result) == 0);
899-
}
900-
901-
/* Check a Write outcome and report any error. */
902-
#define CW(cond) \
903-
do { \
904-
if (!(cond)) \
905-
{ \
906-
fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"), \
907-
progname, fname, strerror(errno)); \
908-
exit(2); \
909-
} \
910-
} while (0)
911-
912-
res = snprintf(fname, sizeof(fname), "%s/pg_hba.conf", pgdata);
913-
if (res < 0 || res >= sizeof(fname))
914-
{
915-
/*
916-
* Truncating this name is a fatal error, because we must not fail to
917-
* overwrite an original trust-authentication pg_hba.conf.
918-
*/
919-
fprintf(stderr, _("%s: directory name too long\n"), progname);
920-
exit(2);
921-
}
922-
hba = fopen(fname, "w");
923-
if (hba == NULL)
924-
{
925-
fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
926-
progname, fname, strerror(errno));
927-
exit(2);
928-
}
929-
CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
930-
CW(fputs("host all all 127.0.0.1/32 sspi include_realm=1 map=regress\n",
931-
hba) >= 0);
932-
if (have_ipv6)
933-
CW(fputs("host all all ::1/128 sspi include_realm=1 map=regress\n",
934-
hba) >= 0);
935-
CW(fclose(hba) == 0);
936-
937-
snprintf(fname, sizeof(fname), "%s/pg_ident.conf", pgdata);
938-
ident = fopen(fname, "w");
939-
if (ident == NULL)
940-
{
941-
fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
942-
progname, fname, strerror(errno));
943-
exit(2);
944-
}
945-
CW(fputs("# Configuration written by config_sspi_auth()\n", ident) >= 0);
946-
947-
/*
948-
* Double-quote for the benefit of account names containing whitespace or
949-
* '#'. Windows forbids the double-quote character itself, so don't
950-
* bother escaping embedded double-quote characters.
951-
*/
952-
CW(fprintf(ident, "regress \"%s@%s\" %s\n",
953-
accountname, domainname, fmtHba(superuser_name)) >= 0);
954-
for (sl = extraroles; sl; sl = sl->next)
955-
CW(fprintf(ident, "regress \"%s@%s\" %s\n",
956-
accountname, domainname, fmtHba(sl->str)) >= 0);
957-
CW(fclose(ident) == 0);
958-
}
959-
960-
#endif /* ENABLE_SSPI */
961-
962785
/*
963786
* psql_start_command, psql_add_command, psql_end_command
964787
*
@@ -2039,7 +1862,6 @@ regression_main(int argc, char *argv[],
20391862
{NULL, 0, NULL, 0}
20401863
};
20411864

2042-
bool use_unix_sockets;
20431865
_stringlist *sl;
20441866
int c;
20451867
int i;
@@ -2055,20 +1877,6 @@ regression_main(int argc, char *argv[],
20551877

20561878
atexit(stop_postmaster);
20571879

2058-
#if defined(WIN32)
2059-
2060-
/*
2061-
* We don't use Unix-domain sockets on Windows by default (see comment at
2062-
* remove_temp() for a reason). Override at your own risk.
2063-
*/
2064-
use_unix_sockets = getenv("PG_TEST_USE_UNIX_SOCKETS") ? true : false;
2065-
#else
2066-
use_unix_sockets = true;
2067-
#endif
2068-
2069-
if (!use_unix_sockets)
2070-
hostname = "localhost";
2071-
20721880
/*
20731881
* We call the initialization function here because that way we can set
20741882
* default parameters and let them be overwritten by the commandline.
@@ -2194,13 +2002,7 @@ regression_main(int argc, char *argv[],
21942002
}
21952003

21962004
if (config_auth_datadir)
2197-
{
2198-
#ifdef ENABLE_SSPI
2199-
if (!use_unix_sockets)
2200-
config_sspi_auth(config_auth_datadir, user);
2201-
#endif
22022005
exit(0);
2203-
}
22042006

22052007
if (temp_instance && !port_specified_by_user)
22062008

@@ -2319,18 +2121,6 @@ regression_main(int argc, char *argv[],
23192121

23202122
fclose(pg_conf);
23212123

2322-
#ifdef ENABLE_SSPI
2323-
if (!use_unix_sockets)
2324-
{
2325-
/*
2326-
* Since we successfully used the same buffer for the much-longer
2327-
* "initdb" command, this can't truncate.
2328-
*/
2329-
snprintf 5DF0 (buf, sizeof(buf), "%s/data", temp_instance);
2330-
config_sspi_auth(buf, NULL);
2331-
}
2332-
#endif
2333-
23342124
/*
23352125
* Check if there is a postmaster running already.
23362126
*/

0 commit comments

Comments
 (0)
0