Commit 48d67fd8 authored by Tom Lane's avatar Tom Lane

Fix race condition in psql \e's detection of file modification.

psql's editing commands decide whether the user has edited the file
by checking for change of modification timestamp.  This is probably
fine for a pre-existing file, but with a temporary file that is
created within the command, it's possible for a fast typist to
save-and-exit in less than the one-second granularity of stat(2)
timestamps.  On Windows FAT filesystems the granularity is even
worse, 2 seconds, making the race a bit easier to hit.

To fix, try to set the temp file's mod time to be two seconds ago.
It's unlikely this would fail, but then again the race condition
itself is unlikely, so just ignore any error.

Also, we might as well check the file size as well as its mod time.

While this is a difficult bug to hit, it still seems worth
back-patching, to ensure that users' edits aren't lost.

Laurenz Albe, per gripe from Jacob Champion; based on fix suggestions
from Jacob and myself

Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel@cybertec.at
parent f52c5d67
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <ctype.h> #include <ctype.h>
#include <time.h> #include <time.h>
#include <pwd.h> #include <pwd.h>
#include <utime.h>
#ifndef WIN32 #ifndef WIN32
#include <sys/stat.h> /* for stat() */ #include <sys/stat.h> /* for stat() */
#include <fcntl.h> /* open() flags */ #include <fcntl.h> /* open() flags */
...@@ -3709,7 +3710,6 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, ...@@ -3709,7 +3710,6 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
const char *fname; const char *fname;
bool error = false; bool error = false;
int fd; int fd;
struct stat before, struct stat before,
after; after;
...@@ -3734,13 +3734,13 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, ...@@ -3734,13 +3734,13 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
!ret ? strerror(errno) : ""); !ret ? strerror(errno) : "");
return false; return false;
} }
#endif
/* /*
* No canonicalize_path() here. EDIT.EXE run from CMD.EXE prepends the * No canonicalize_path() here. EDIT.EXE run from CMD.EXE prepends the
* current directory to the supplied path unless we use only * current directory to the supplied path unless we use only
* backslashes, so we do that. * backslashes, so we do that.
*/ */
#endif
#ifndef WIN32 #ifndef WIN32
snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d.sql", tmpdir, snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d.sql", tmpdir,
"/", (int) getpid()); "/", (int) getpid());
...@@ -3790,6 +3790,24 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, ...@@ -3790,6 +3790,24 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
pg_log_error("%s: %m", fname); pg_log_error("%s: %m", fname);
error = true; error = true;
} }
else
{
struct utimbuf ut;
/*
* Try to set the file modification time of the temporary file
* a few seconds in the past. Otherwise, the low granularity
* (one second, or even worse on some filesystems) that we can
* portably measure with stat(2) could lead us to not
* recognize a modification, if the user typed very quickly.
*
* This is a rather unlikely race condition, so don't error
* out if the utime(2) call fails --- that would make the cure
* worse than the disease.
*/
ut.modtime = ut.actime = time(NULL) - 2;
(void) utime(fname, &ut);
}
} }
} }
...@@ -3809,7 +3827,10 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, ...@@ -3809,7 +3827,10 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
error = true; error = true;
} }
if (!error && before.st_mtime != after.st_mtime) /* file was edited if the size or modification time has changed */
if (!error &&
(before.st_size != after.st_size ||
before.st_mtime != after.st_mtime))
{ {
stream = fopen(fname, PG_BINARY_R); stream = fopen(fname, PG_BINARY_R);
if (!stream) if (!stream)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment