Fix detection of unseekable files for fseek() and ftello() with MSVC
authorMichael Paquier
Wed, 12 Apr 2023 00:09:53 +0000 (09:09 +0900)
committerMichael Paquier
Wed, 12 Apr 2023 00:09:53 +0000 (09:09 +0900)
Calling fseek() or ftello() on a handle to a non-seeking device such as
a pipe or a communications device is not supported.  Unfortunately,
MSVC's flavor of these routines, _fseeki64() and _ftelli64(), do not
return an error when given a pipe as handle.  Some of the logic of
pg_dump and restore relies on these routines to check if a handle is
seekable, causing failures when passing the contents of pg_dump to
pg_restore through a pipe, for example.

This commit introduces wrappers for fseeko() and ftello() on MSVC so as
any callers are able to properly detect the cases of non-seekable
handles.  This relies mainly on GetFileType(), sharing a bit of code
with the MSVC port for fstat().  The code in charge of getting a file
type is refactored into a new file called win32common.c, shared by
win32stat.c and the new win32fseek.c.  It includes the MSVC ports for
fseeko() and ftello().

Like 765f5df, this is backpatched down to 14, where the fstat()
implementation for MSVC is able to understand about files larger than
4GB in size.  Using a TAP test for that is proving to be tricky as
IPC::Run handles the pipes by itself, still I have been able to check
the fix manually.

Reported-by: Daniel Watzinger
Author: Juan José Santamaría Flecha, Michael Paquier
Discussion: https://postgr.es/m/CAC+AXB26a4EmxM2suXxPpJaGrqAdxracd7hskLg-zxtPB50h7A@mail.gmail.com
Backpatch-through: 14

configure
configure.ac
src/include/port/win32_port.h
src/port/win32common.c [new file with mode: 0644]
src/port/win32fseek.c [new file with mode: 0644]
src/port/win32stat.c
src/tools/msvc/Mkvcbuild.pm

index 6c87ca479a87d0499e400cd4902b5d6a97a423dc..c117157d9a3a61f98dda85e120a17d4b7615d3d5 100755 (executable)
--- a/configure
+++ b/configure
@@ -17287,6 +17287,12 @@ esac
  ;;
 esac
 
+  case " $LIBOBJS " in
+  *" win32common.$ac_objext "* ) ;;
+  *) LIBOBJS="$LIBOBJS win32common.$ac_objext"
+ ;;
+esac
+
   case " $LIBOBJS " in
   *" win32env.$ac_objext "* ) ;;
   *) LIBOBJS="$LIBOBJS win32env.$ac_objext"
index 01968035f3d41b68b2ec07dc97b04bf9c0037671..e43a688847152024e16552fe26da8f6a3a93df10 100644 (file)
@@ -2004,6 +2004,7 @@ if test "$PORTNAME" = "win32"; then
   AC_LIBOBJ(kill)
   AC_LIBOBJ(open)
   AC_LIBOBJ(system)
+  AC_LIBOBJ(win32common)
   AC_LIBOBJ(win32env)
   AC_LIBOBJ(win32error)
   AC_LIBOBJ(win32ntdll)
index dbbf88f8e8cb1d695bb32e96ef2957fa101ba201..d1e89f364ac8732f3663af933b2a502c0b3ab798 100644 (file)
@@ -201,15 +201,21 @@ struct itimerval
 
 int            setitimer(int which, const struct itimerval *value, struct itimerval *ovalue);
 
+/* Convenience wrapper for GetFileType() */
+extern DWORD pgwin32_get_file_type(HANDLE hFile);
+
 /*
  * WIN32 does not provide 64-bit off_t, but does provide the functions operating
- * with 64-bit offsets.
+ * with 64-bit offsets.  Also, fseek() might not give an error for unseekable
+ * streams, so harden that function with our version.
  */
 #define pgoff_t __int64
 
 #ifdef _MSC_VER
-#define fseeko(stream, offset, origin) _fseeki64(stream, offset, origin)
-#define ftello(stream) _ftelli64(stream)
+extern int _pgfseeko64(FILE *stream, pgoff_t offset, int origin);
+extern pgoff_t _pgftello64(FILE *stream);
+#define fseeko(stream, offset, origin) _pgfseeko64(stream, offset, origin)
+#define ftello(stream) _pgftello64(stream)
 #else
 #ifndef fseeko
 #define fseeko(stream, offset, origin) fseeko64(stream, offset, origin)
diff --git a/src/port/win32common.c b/src/port/win32common.c
new file mode 100644 (file)
index 0000000..2fd78f7
--- /dev/null
@@ -0,0 +1,68 @@
+/*-------------------------------------------------------------------------
+ *
+ * win32common.c
+ *   Common routines shared among the win32*.c ports.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *   src/port/win32common.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
+#include "postgres.h"
+#endif
+
+#ifdef WIN32
+
+/*
+ * pgwin32_get_file_type
+ *
+ * Convenience wrapper for GetFileType() with specific error handling for all the
+ * port implementations.  Returns the file type associated with a HANDLE.
+ *
+ * On error, sets errno with FILE_TYPE_UNKNOWN as file type.
+ */
+DWORD
+pgwin32_get_file_type(HANDLE hFile)
+{
+   DWORD       fileType = FILE_TYPE_UNKNOWN;
+   DWORD       lastError;
+
+   errno = 0;
+
+   /*
+    * When stdin, stdout, and stderr aren't associated with a stream the
+    * special value -2 is returned:
+    * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle
+    */
+   if (hFile == INVALID_HANDLE_VALUE || hFile == (HANDLE) -2)
+   {
+       errno = EINVAL;
+       return FILE_TYPE_UNKNOWN;
+   }
+
+   fileType = GetFileType(hFile);
+   lastError = GetLastError();
+
+   /*
+    * Invoke GetLastError in order to distinguish between a "valid" return of
+    * FILE_TYPE_UNKNOWN and its return due to a calling error.  In case of
+    * success, GetLastError() returns NO_ERROR.
+    */
+   if (fileType == FILE_TYPE_UNKNOWN && lastError != NO_ERROR)
+   {
+       _dosmaperr(lastError);
+       return FILE_TYPE_UNKNOWN;
+   }
+
+   return fileType;
+}
+
+#endif                         /* WIN32 */
diff --git a/src/port/win32fseek.c b/src/port/win32fseek.c
new file mode 100644 (file)
index 0000000..985313c
--- /dev/null
@@ -0,0 +1,75 @@
+/*-------------------------------------------------------------------------
+ *
+ * win32fseek.c
+ *   Replacements for fseeko() and ftello().
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *   src/port/win32fseek.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
+#include "postgres.h"
+#endif
+
+#if defined(WIN32) && defined(_MSC_VER)
+
+/*
+ * _pgfseeko64
+ *
+ * Calling fseek() on a handle to a non-seeking device such as a pipe or
+ * a communications device is not supported, and fseek() may not return
+ * an error.  This wrapper relies on the file type to check which cases
+ * are supported.
+ */
+int
+_pgfseeko64(FILE *stream, pgoff_t offset, int origin)
+{
+   DWORD       fileType;
+   HANDLE      hFile = (HANDLE) _get_osfhandle(_fileno(stream));
+
+   fileType = pgwin32_get_file_type(hFile);
+   if (errno != 0)
+       return -1;
+
+   if (fileType == FILE_TYPE_DISK)
+       return _fseeki64(stream, offset, origin);
+   else if (fileType == FILE_TYPE_CHAR || fileType == FILE_TYPE_PIPE)
+       errno = ESPIPE;
+   else
+       errno = EINVAL;
+
+   return -1;
+}
+
+/*
+ * _pgftello64
+ *
+ * Same as _pgfseeko64().
+ */
+pgoff_t
+_pgftello64(FILE *stream)
+{
+   DWORD       fileType;
+   HANDLE      hFile = (HANDLE) _get_osfhandle(_fileno(stream));
+
+   fileType = pgwin32_get_file_type(hFile);
+   if (errno != 0)
+       return -1;
+
+   if (fileType == FILE_TYPE_DISK)
+       return _ftelli64(stream);
+   else if (fileType == FILE_TYPE_CHAR || fileType == FILE_TYPE_PIPE)
+       errno = ESPIPE;
+   else
+       errno = EINVAL;
+
+   return -1;
+}
+
+#endif                         /* defined(WIN32) && defined(_MSC_VER) */
index a38dce256d5763a0d38a7b14065d7eb79ef9b83d..e25811d290e44ba150f8fd496f40b438e95e10ab 100644 (file)
@@ -141,33 +141,17 @@ _pgfstat64(int fileno, struct stat *buf)
 {
    HANDLE      hFile = (HANDLE) _get_osfhandle(fileno);
    DWORD       fileType = FILE_TYPE_UNKNOWN;
-   DWORD       lastError;
    unsigned short st_mode;
 
-   /*
-    * When stdin, stdout, and stderr aren't associated with a stream the
-    * special value -2 is returned:
-    * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle
-    */
-   if (hFile == INVALID_HANDLE_VALUE || hFile == (HANDLE) -2 || buf == NULL)
+   if (buf == NULL)
    {
        errno = EINVAL;
        return -1;
    }
 
-   fileType = GetFileType(hFile);
-   lastError = GetLastError();
-
-   /*
-    * Invoke GetLastError in order to distinguish between a "valid" return of
-    * FILE_TYPE_UNKNOWN and its return due to a calling error.  In case of
-    * success, GetLastError returns NO_ERROR.
-    */
-   if (fileType == FILE_TYPE_UNKNOWN && lastError != NO_ERROR)
-   {
-       _dosmaperr(lastError);
+   fileType = pgwin32_get_file_type(hFile);
+   if (errno != 0)
        return -1;
-   }
 
    switch (fileType)
    {
index ef0a33c10f1b479abc573fa9192634ffc610e3e9..990c223a9b4e04b0bdb9cbfa78c4a9ad09beca1a 100644 (file)
@@ -107,7 +107,7 @@ sub mkvcbuild
      pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c
      pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c
      strerror.c tar.c
-     win32env.c win32error.c win32ntdll.c
+     win32common.c win32env.c win32error.c win32fseek.c win32ntdll.c
      win32security.c win32setlocale.c win32stat.c);
 
    push(@pgportfiles, 'strtof.c') if ($vsVersion < '14.00');