From 6f767ac04b53e7070a3b4c758e8c85552456184f Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Wed, 18 Mar 2020 13:48:05 +0100 Subject: [PATCH] =?UTF-8?q?Revert:=20Semmle:=20Protect=20against=20theoret?= =?UTF-8?q?ical=20race=20in=20recursive=20d=E2=80=A6=20(#3619)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As discussed with @JelteF; #3559 caused consistent errors on BSD (OSX). Given a group of people use this environment to develop on it is an undesirable change. This reverts commit ca8f7119fe6defe431db4528c42c1ac7f317846f. (cherry picked from commit 6ff79c5ea96d830b1ec9a146079c93cfd7f8ca51) --- .../worker/worker_partition_protocol.c | 165 +++++++++--------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/src/backend/distributed/worker/worker_partition_protocol.c b/src/backend/distributed/worker/worker_partition_protocol.c index 7f14ad42f..2aa4bcf31 100644 --- a/src/backend/distributed/worker/worker_partition_protocol.c +++ b/src/backend/distributed/worker/worker_partition_protocol.c @@ -85,8 +85,7 @@ static uint32 RangePartitionId(Datum partitionValue, Oid partitionCollation, static uint32 HashPartitionId(Datum partitionValue, Oid partitionCollation, const void *context); static StringInfo UserPartitionFilename(StringInfo directoryName, uint32 partitionId); -static bool TryUnlink(const char *filename); -static bool TryRmdir(const char *filename); +static bool FileIsLink(const char *filename, struct stat filestat); /* exports for SQL callable functions */ @@ -689,6 +688,25 @@ CitusCreateDirectory(StringInfo directoryName) } +#ifdef WIN32 +static bool +FileIsLink(char *filename, struct stat filestat) +{ + return pgwin32_is_junction(filename); +} + + +#else +static bool +FileIsLink(const char *filename, struct stat filestat) +{ + return S_ISLNK(filestat.st_mode); +} + + +#endif + + /* * CitusRemoveDirectory first checks if the given directory exists. If it does, the * function recursively deletes the contents of the given directory, and then @@ -701,102 +719,87 @@ CitusRemoveDirectory(const char *filename) /* files may be added during execution, loop when that occurs */ while (true) { - if (TryUnlink(filename) || TryRmdir(filename)) + struct stat fileStat; + int removed = 0; + + int statOK = stat(filename, &fileStat); + if (statOK < 0) { - return; + if (errno == ENOENT) + { + return; /* if file does not exist, return */ + } + else + { + ereport(ERROR, (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", filename))); + } } /* - * If both of these failed then filename is a non empty directory. - * Iterate over all its contents and for each content, recurse into - * this function. Also, make sure that we do not recurse into symbolic - * links. + * If this is a directory, iterate over all its contents and for each + * content, recurse into this function. Also, make sure that we do not + * recurse into symbolic links. */ - const char *directoryName = filename; + if (S_ISDIR(fileStat.st_mode) && !FileIsLink(filename, fileStat)) + { + const char *directoryName = filename; - DIR *directory = AllocateDir(directoryName); - if (errno == ENOENT) - { - /* If directory was removed from under us we're done */ - return; - } - if (errno == ENOTDIR) - { - /* If directory changed to a file underneath us, loop again to remove it with unlink */ - continue; + DIR *directory = AllocateDir(directoryName); + if (directory == NULL) + { + ereport(ERROR, (errcode_for_file_access(), + errmsg("could not open directory \"%s\": %m", + directoryName))); + } + + StringInfo fullFilename = makeStringInfo(); + struct dirent *directoryEntry = ReadDir(directory, directoryName); + for (; directoryEntry != NULL; directoryEntry = ReadDir(directory, + directoryName)) + { + const char *baseFilename = directoryEntry->d_name; + + /* if system file, skip it */ + if (strncmp(baseFilename, ".", MAXPGPATH) == 0 || + strncmp(baseFilename, "..", MAXPGPATH) == 0) + { + continue; + } + + resetStringInfo(fullFilename); + appendStringInfo(fullFilename, "%s/%s", directoryName, baseFilename); + + CitusRemoveDirectory(fullFilename->data); + } + + FreeStringInfo(fullFilename); + FreeDir(directory); } - /* ReadDir handles other errno from AllocateDir */ - struct dirent *directoryEntry = ReadDir(directory, directoryName); - StringInfo fullFilename = makeStringInfo(); - for (; directoryEntry != NULL; directoryEntry = ReadDir(directory, - directoryName)) + /* we now have an empty directory or a regular file, remove it */ + if (S_ISDIR(fileStat.st_mode)) { - const char *baseFilename = directoryEntry->d_name; + removed = rmdir(filename); - /* if system file, skip it */ - if (strncmp(baseFilename, ".", MAXPGPATH) == 0 || - strncmp(baseFilename, "..", MAXPGPATH) == 0) + if (errno == ENOTEMPTY || errno == EEXIST) { continue; } - - resetStringInfo(fullFilename); - appendStringInfo(fullFilename, "%s/%s", directoryName, baseFilename); - - CitusRemoveDirectory(fullFilename->data); + } + else + { + removed = unlink(filename); } - FreeStringInfo(fullFilename); - FreeDir(directory); - } -} + if (removed != 0 && errno != ENOENT) + { + ereport(ERROR, (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", filename))); + } - -/* - * TryUnlink returns true if file is removed and false if the file is a - * directory. If an error occurs trying to remove the file this function calls - * ereport. - */ -static bool -TryUnlink(const char *filename) -{ - errno = 0; - int exitcode = unlink(filename); - if (exitcode == 0 || errno == ENOENT) - { - return true; + return; } - if (errno == EISDIR) - { - return false; - } - ereport(ERROR, (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", filename))); -} - - -/* - * TryRmdir returns true if the directory is removed. It returns false if the - * filename points to a file or symlink. It also returns false if the directory - * is not empty. The difference in these cases can be found by checking errno. - * For all other errors ereport is called. - */ -static bool -TryRmdir(const char *filename) -{ - errno = 0; - int exitcode = rmdir(filename); - if (exitcode == 0 || errno == ENOENT) - { - return true; - } - if (errno == ENOTDIR || errno == ENOTEMPTY || errno == EEXIST) - { - return false; - } - ereport(ERROR, (errcode_for_file_access(), - errmsg("could not remove directory \"%s\": %m", filename))); }