Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 60e5c2b

Browse files
committed
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
1 parent e7f7950 commit 60e5c2b

File tree

1 file changed

+24
-3
lines changed

1 file changed

+24
-3
lines changed

src/bin/psql/command.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <ctype.h>
1212
#include <time.h>
1313
#include <pwd.h>
14+
#include <utime.h>
1415
#ifndef WIN32
1516
#include <sys/stat.h> /* for stat() */
1617
#include <fcntl.h> /* open() flags */
@@ -3565,7 +3566,6 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
35653566
const char *fname;
35663567
bool error = false;
35673568
int fd;
3568-
35693569
struct stat before,
35703570
after;
35713571

@@ -3590,13 +3590,13 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
35903590
!ret ? strerror(errno) : "");
35913591
return false;
35923592
}
3593+
#endif
35933594

35943595
/*
35953596
* No canonicalize_path() here. EDIT.EXE run from CMD.EXE prepends the
35963597
* current directory to the supplied path unless we use only
35973598
* backslashes, so we do that.
35983599
*/
3599-
#endif
36003600
#ifndef WIN32
36013601
snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d.sql", tmpdir,
36023602
"/", (int) getpid());
@@ -3645,6 +3645,24 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
36453645
pg_log_error("%s: %m", fname);
36463646
error = true;
36473647
}
3648+
else
3649+
{
3650+
struct utimbuf ut;
3651+
3652+
/*
3653+
* Try to set the file modification time of the temporary file
3654+
* a few seconds in the past. Otherwise, the low granularity
3655+
* (one second, or even worse on some filesystems) that we can
3656+
* portably measure with stat(2) could lead us to not
3657+
* recognize a modification, if the user typed very quickly.
3658+
*
3659+
* This is a rather unlikely race condition, so don't error
3660+
* out if the utime(2) call fails --- that would make the cure
3661+
* worse than the disease.
3662+
*/
3663+
ut.modtime = ut.actime = time(NULL) - 2;
3664+
(void) utime(fname, &ut);
3665+
}
36483666
}
36493667
}
36503668

@@ -3664,7 +3682,10 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
36643682
error = true;
36653683
}
36663684

3667-
if (!error && before.st_mtime != after.st_mtime)
3685+
/* file was edited if the size or modification time has changed */
3686+
if (!error &&
3687+
(before.st_size != after.st_size ||
3688+
before.st_mtime != after.st_mtime))
36683689
{
36693690
stream = fopen(fname, PG_BINARY_R);
36703691
if (!stream)

0 commit comments

Comments
 (0)