From 1a442bfde9b44d28db7344df56e5c0069dbd2364 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 25 May 2021 20:42:17 +1200 Subject: [PATCH v7 1/5] Allow restricted relative tablespace paths. Traditionally, tablespace paths provided to LOCATION had to be absolute, because relative paths risked colliding with PostgreSQL-managed files and generally blurring the boundary between system-managed and user-managed data. This commit adds a very small exception: it allows relative paths to be used, but only under a new directory "pg_user_files", and only if the developer-only option allow_relative_tablespaces is enabled. This is intended to be used in automated testing. Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com --- src/backend/commands/tablespace.c | 55 +++++++++++++++++++++++++++---- src/backend/utils/misc/guc.c | 12 +++++++ src/bin/initdb/initdb.c | 1 + src/common/string.c | 9 +++++ src/include/commands/tablespace.h | 2 ++ src/include/common/string.h | 1 + src/port/dirmod.c | 4 +++ 7 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 4b96eec9df..7e98ca5b76 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -70,6 +70,7 @@ #include "commands/tablecmds.h" #include "commands/tablespace.h" #include "common/file_perm.h" +#include "common/string.h" #include "miscadmin.h" #include "postmaster/bgwriter.h" #include "storage/fd.h" @@ -87,6 +88,7 @@ /* GUC variables */ char *default_tablespace = NULL; char *temp_tablespaces = NULL; +bool allow_relative_tablespaces = false; static void create_tablespace_directories(const char *location, @@ -267,11 +269,15 @@ CreateTableSpace(CreateTableSpaceStmt *stmt) errmsg("tablespace location cannot contain single quotes"))); /* - * Allowing relative paths seems risky + * Allowing relative paths seems risky in general, but we'll allow it under + * pg_user_files if a developer-mode option is enabled, for the purpose + * of testing. * * this also helps us ensure that location is not empty or whitespace */ - if (!is_absolute_path(location)) + if (!is_absolute_path(location) && + (!allow_relative_tablespaces || + !pg_str_startswith(location, "pg_user_files/"))) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("tablespace location must be an absolute path"))); @@ -587,6 +593,7 @@ DropTableSpace(DropTableSpaceStmt *stmt) static void create_tablespace_directories(const char *location, const Oid tablespaceoid) { + char buffer[MAXPGPATH]; char *linkloc; char *location_with_version_dir; struct stat st; @@ -602,11 +609,27 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) if (chmod(location, pg_dir_create_mode) != 0) { if (errno == ENOENT) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FILE), - errmsg("directory \"%s\" does not exist", location), - InRecovery ? errhint("Create this directory for the tablespace before " - "restarting the server.") : 0)); + { + /* + * We're prepared to create directories under pg_user_files on the + * fly, for the benefit of regression tests. + */ + if (pg_str_startswith(location, "pg_user_files/")) + { + Assert(allow_relative_tablespaces || InRecovery); + if (mkdir(location, pg_dir_create_mode) != 0) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FILE), + errmsg("could not create directory \"%s\"", + location))); + } + else + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FILE), + errmsg("directory \"%s\" does not exist", location), + InRecovery ? errhint("Create this directory for the tablespace before " + "restarting the server.") : 0)); + } else ereport(ERROR, (errcode_for_file_access(), @@ -651,6 +674,24 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) if (InRecovery) remove_tablespace_symlink(linkloc); + if (!is_absolute_path(location)) + { +#ifdef WIN32 + /* + * Our Windows junction-based symlink() emulation can't handle relative + * paths, so convert "pg_user_files" to an absolute path. + */ + snprintf(buffer, sizeof(buffer), "%s/%s", DataDir, location); +#else + /* + * Add a ".." prefix so that we have a path relative to the symlink, + * rather than DataDir. + */ + snprintf(buffer, sizeof(buffer), "../%s", location); +#endif + location = buffer; + } + /* * Create the symlink under PGDATA */ diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ee6a838b3a..ab11bee118 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -46,6 +46,7 @@ #include "catalog/storage.h" #include "commands/async.h" #include "commands/prepare.h" +#include "commands/tablespace.h" #include "commands/trigger.h" #include "commands/user.h" #include "commands/vacuum.h" @@ -1963,6 +1964,17 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"allow_relative_tablespaces", PGC_SUSET, DEVELOPER_OPTIONS, + gettext_noop("Allows the tablespaces located under pg_user_files."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &allow_relative_tablespaces, + false, + NULL, NULL, NULL + }, + { {"lo_compat_privileges", PGC_SUSET, COMPAT_OPTIONS_PREVIOUS, gettext_noop("Enables backward compatibility mode for privilege checks on large objects."), diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 403adb0ee7..7c4b3e9626 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -225,6 +225,7 @@ static const char *const subdirs[] = { "pg_tblspc", "pg_stat", "pg_stat_tmp", + "pg_user_files", "pg_xact", "pg_logical", "pg_logical/snapshots", diff --git a/src/common/string.c b/src/common/string.c index 3aa378c051..7e7e34f1f5 100644 --- a/src/common/string.c +++ b/src/common/string.c @@ -24,6 +24,15 @@ #include "common/string.h" +/* + * Returns whether the string `str' has the prefix `start'. + */ +bool +pg_str_startswith(const char *str, const char *start) +{ + return strncmp(str, start, strlen(start)) == 0; +} + /* * Returns whether the string `str' has the postfix `end'. */ diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h index 49cfc8479a..d286ced1c8 100644 --- a/src/include/commands/tablespace.h +++ b/src/include/commands/tablespace.h @@ -19,6 +19,8 @@ #include "lib/stringinfo.h" #include "nodes/parsenodes.h" +extern bool allow_relative_tablespaces; + /* XLOG stuff */ #define XLOG_TBLSPC_CREATE 0x00 #define XLOG_TBLSPC_DROP 0x10 diff --git a/src/include/common/string.h b/src/include/common/string.h index 8eb5271ec8..ddd384f6f9 100644 --- a/src/include/common/string.h +++ b/src/include/common/string.h @@ -21,6 +21,7 @@ typedef struct PromptInterruptContext } PromptInterruptContext; /* functions in src/common/string.c */ +extern bool pg_str_startswith(const char *str, const char *start); extern bool pg_str_endswith(const char *str, const char *end); extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base); diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..8787912d03 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -182,6 +182,10 @@ pgsymlink(const char *oldpath, const char *newpath) while ((p = strchr(p, '/')) != NULL) *p++ = '\\'; + /* collapse useless instances of "\.\" to "\" */ + while ((p = strstr(nativeTarget, "\\.\\")) != NULL) + memmove(p, p + 2, strlen(p + 2) + 1); + len = strlen(nativeTarget) * sizeof(WCHAR); reparseBuf->ReparseTag = IO_REPARSE_TAG_MOUNT_POINT; reparseBuf->ReparseDataLength = len + 12; -- 2.33.1