From a967613911f7ef7b6387b9e8718f0ab8f0c4d9c8 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 7 Jan 2016 11:59:08 -0300 Subject: [PATCH] Windows: Make pg_ctl reliably detect service status pg_ctl is using isatty() to verify whether the process is running in a terminal, and if not it sends its output to Windows' Event Log ... which does the wrong thing when the output has been redirected to a pipe, as reported in bug #13592. To fix, make pg_ctl use the code we already have to detect service-ness: in the master branch, move src/backend/port/win32/security.c to src/port (with suitable tweaks so that it runs properly in backend and frontend environments); pg_ctl already has access to pgport so it Just Works. In older branches, that's likely to cause trouble, so instead duplicate the required code in pg_ctl.c. Author: Michael Paquier Bug report and diagnosis: Egon Kocjan Backpatch: all supported branches --- src/backend/port/win32/Makefile | 2 +- src/bin/pg_ctl/pg_ctl.c | 2 +- src/include/port/win32.h | 7 +- .../win32/security.c => port/win32security.c} | 78 +++++++++++++------ src/tools/msvc/Mkvcbuild.pm | 2 +- 5 files changed, 63 insertions(+), 28 deletions(-) rename src/{backend/port/win32/security.c => port/win32security.c} (73%) diff --git a/src/backend/port/win32/Makefile b/src/backend/port/win32/Makefile index 820a3b3753..a6ace93e26 100644 --- a/src/backend/port/win32/Makefile +++ b/src/backend/port/win32/Makefile @@ -12,7 +12,7 @@ subdir = src/backend/port/win32 top_builddir = ../../../.. include $(top_builddir)/src/Makefile.global -OBJS = timer.o socket.o signal.o security.o mingwcompat.o +OBJS = timer.o socket.o signal.o mingwcompat.o ifeq ($(have_win32_dbghelp), yes) OBJS += crashdump.o endif diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index bd82a059a0..28d3cf2a8f 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -216,7 +216,7 @@ write_stderr(const char *fmt,...) * On Win32, we print to stderr if running on a console, or write to * eventlog if running as a service */ - if (!isatty(fileno(stderr))) /* Running as a service */ + if (!pgwin32_is_service()) /* Running as a service */ { char errbuf[2048]; /* Arbitrary size? */ diff --git a/src/include/port/win32.h b/src/include/port/win32.h index 4cb51ec7be..69cd1c41e1 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -382,9 +382,6 @@ int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout); extern int pgwin32_noblock; -/* in backend/port/win32/security.c */ -extern int pgwin32_is_admin(void); -extern int pgwin32_is_service(void); #endif /* in backend/port/win32_shmem.c */ @@ -400,6 +397,10 @@ extern void _dosmaperr(unsigned long); extern int pgwin32_putenv(const char *); extern void pgwin32_unsetenv(const char *); +/* in port/win32security.c */ +extern int pgwin32_is_service(void); +extern int pgwin32_is_admin(void); + #define putenv(x) pgwin32_putenv(x) #define unsetenv(x) pgwin32_unsetenv(x) diff --git a/src/backend/port/win32/security.c b/src/port/win32security.c similarity index 73% rename from src/backend/port/win32/security.c rename to src/port/win32security.c index 327ed404fd..66796e0e28 100644 --- a/src/backend/port/win32/security.c +++ b/src/port/win32security.c @@ -1,22 +1,47 @@ /*------------------------------------------------------------------------- * - * security.c + * win32security.c * Microsoft Windows Win32 Security Support Functions * * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group * * IDENTIFICATION - * src/backend/port/win32/security.c + * src/port/win32security.c * *------------------------------------------------------------------------- */ +#ifndef FRONTEND #include "postgres.h" +#else +#include "postgres_fe.h" +#endif static BOOL pgwin32_get_dynamic_tokeninfo(HANDLE token, - TOKEN_INFORMATION_CLASS class, char **InfoBuffer, - char *errbuf, int errsize); + TOKEN_INFORMATION_CLASS class, + char **InfoBuffer, char *errbuf, int errsize); + + +/* + * Utility wrapper for frontend and backend when reporting an error + * message. + */ +static +pg_attribute_printf(1, 2) +void +log_error(const char *fmt,...) +{ + va_list ap; + + va_start(fmt, ap); +#ifndef FRONTEND + write_stderr(fmt, ap); +#else + fprintf(stderr, fmt, ap); +#endif + va_end(ap); +} /* * Returns nonzero if the current user has administrative privileges, @@ -40,15 +65,15 @@ pgwin32_is_admin(void) if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &AccessToken)) { - write_stderr("could not open process token: error code %lu\n", - GetLastError()); + log_error("could not open process token: error code %lu\n", + GetLastError()); exit(1); } if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups, &InfoBuffer, errbuf, sizeof(errbuf))) { - write_stderr("%s", errbuf); + log_error("%s", errbuf); exit(1); } @@ -57,20 +82,22 @@ pgwin32_is_admin(void) CloseHandle(AccessToken); if (!AllocateAndInitializeSid(&NtAuthority, 2, - SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, + SECURITY_BUILTIN_DOMAIN_RID, + DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, &AdministratorsSid)) { - write_stderr("could not get SID for Administrators group: error code %lu\n", - GetLastError()); + log_error("could not get SID for Administrators group: error code %lu\n", + GetLastError()); exit(1); } if (!AllocateAndInitializeSid(&NtAuthority, 2, - SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_POWER_USERS, 0, 0, 0, 0, 0, + SECURITY_BUILTIN_DOMAIN_RID, + DOMAIN_ALIAS_RID_POWER_USERS, 0, 0, 0, 0, 0, 0, &PowerUsersSid)) { - write_stderr("could not get SID for PowerUsers group: error code %lu\n", - GetLastError()); + log_error("could not get SID for PowerUsers group: error code %lu\n", + GetLastError()); exit(1); } @@ -78,8 +105,10 @@ pgwin32_is_admin(void) for (x = 0; x < Groups->GroupCount; x++) { - if ((EqualSid(AdministratorsSid, Groups->Groups[x].Sid) && (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)) || - (EqualSid(PowerUsersSid, Groups->Groups[x].Sid) && (Groups->Groups[x].Attributes & SE_GROUP_ENABLED))) + if ((EqualSid(AdministratorsSid, Groups->Groups[x].Sid) && + (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)) || + (EqualSid(PowerUsersSid, Groups->Groups[x].Sid) && + (Groups->Groups[x].Attributes & SE_GROUP_ENABLED))) { success = TRUE; break; @@ -105,9 +134,10 @@ pgwin32_is_admin(void) * 1 = Service * -1 = Error * - * Note: we can't report errors via either ereport (we're called too early) - * or write_stderr (because that calls this). We are therefore reduced to - * writing directly on stderr, which sucks, but we have few alternatives. + * Note: we can't report errors via either ereport (we're called too early + * in the backend) or write_stderr (because that calls this). We are + * therefore reduced to writing directly on stderr, which sucks, but we + * have few alternatives. */ int pgwin32_is_service(void) @@ -217,13 +247,15 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class, if (GetTokenInformation(token, class, NULL, 0, &InfoBufferSize)) { - snprintf(errbuf, errsize, "could not get token information: got zero size\n"); + snprintf(errbuf, errsize, + "could not get token information: got zero size\n"); return FALSE; } if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { - snprintf(errbuf, errsize, "could not get token information: error code %lu\n", + snprintf(errbuf, errsize, + "could not get token information: error code %lu\n", GetLastError()); return FALSE; } @@ -231,7 +263,8 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class, *InfoBuffer = malloc(InfoBufferSize); if (*InfoBuffer == NULL) { - snprintf(errbuf, errsize, "could not allocate %d bytes for token information\n", + snprintf(errbuf, errsize, + "could not allocate %d bytes for token information\n", (int) InfoBufferSize); return FALSE; } @@ -239,7 +272,8 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class, if (!GetTokenInformation(token, class, *InfoBuffer, InfoBufferSize, &InfoBufferSize)) { - snprintf(errbuf, errsize, "could not get token information: error code %lu\n", + snprintf(errbuf, errsize, + "could not get token information: error code %lu\n", GetLastError()); return FALSE; } diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index c122f0259c..1dba7d9662 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -90,7 +90,7 @@ sub mkvcbuild pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c mkdtemp.c qsort.c qsort_arg.c quotes.c system.c sprompt.c tar.c thread.c getopt.c getopt_long.c dirent.c - win32env.c win32error.c win32setlocale.c); + win32env.c win32error.c win32security.c win32setlocale.c); push(@pgportfiles, 'rint.c') if ($vsVersion < '12.00'); -- 2.40.0