From 74686b6de78d19aed6d9aa33a7bb5dfb06f3e018 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 6 Nov 2006 03:06:41 +0000 Subject: [PATCH] Get rid of some unnecessary dependencies on DataDir: wherever possible, the backend should rely on its working-directory setting instead. Also do some message-style police work in contrib/adminpack. --- contrib/adminpack/adminpack.c | 114 ++++++++++++++++---------------- src/backend/utils/adt/dbsize.c | 49 +++++--------- src/backend/utils/adt/genfile.c | 22 +++--- 3 files changed, 83 insertions(+), 102 deletions(-) diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c index 4716dd8150..ab88560c64 100644 --- a/contrib/adminpack/adminpack.c +++ b/contrib/adminpack/adminpack.c @@ -8,7 +8,7 @@ * Author: Andreas Pflug * * IDENTIFICATION - * $PostgreSQL: pgsql/contrib/adminpack/adminpack.c,v 1.7 2006/10/20 00:59:03 tgl Exp $ + * $PostgreSQL: pgsql/contrib/adminpack/adminpack.c,v 1.8 2006/11/06 03:06:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -60,43 +60,47 @@ typedef struct */ /* - * Return an absolute path. Argument may be absolute or - * relative to the DataDir. + * Convert a "text" filename argument to C string, and check it's allowable. + * + * Filename may be absolute or relative to the DataDir, but we only allow + * absolute paths that match DataDir or Log_directory. */ static char * -absClusterPath(text *arg, bool logAllowed) +convert_and_check_filename(text *arg, bool logAllowed) { - char *filename; - int len = VARSIZE(arg) - VARHDRSZ; - int dlen = strlen(DataDir); + int input_len = VARSIZE(arg) - VARHDRSZ; + char *filename = palloc(input_len + 1); - filename = palloc(len + 1); - memcpy(filename, VARDATA(arg), len); - filename[len] = 0; + memcpy(filename, VARDATA(arg), input_len); + filename[input_len] = '\0'; - if (strstr(filename, "..") != NULL) + canonicalize_path(filename); /* filename can change length here */ + + /* Disallow ".." in the path */ + if (path_contains_parent_reference(filename)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("No .. allowed in filenames")))); + (errmsg("reference to parent directory (\"..\") not allowed")))); if (is_absolute_path(filename)) { - if (logAllowed && !strncmp(filename, Log_directory, strlen(Log_directory))) + /* Allow absolute references within DataDir */ + if (path_is_prefix_of_path(DataDir, filename)) + return filename; + /* The log directory might be outside our datadir, but allow it */ + if (logAllowed && + is_absolute_path(Log_directory) && + path_is_prefix_of_path(Log_directory, filename)) return filename; - if (strncmp(filename, DataDir, dlen)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("Absolute path not allowed")))); - return filename; + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("absolute path not allowed")))); + return NULL; /* keep compiler quiet */ } else { - char *absname = palloc(dlen + len + 2); - - sprintf(absname, "%s/%s", DataDir, filename); - pfree(filename); - return absname; + return filename; } } @@ -129,17 +133,17 @@ pg_file_write(PG_FUNCTION_ARGS) requireSuperuser(); - filename = absClusterPath(PG_GETARG_TEXT_P(0), false); + filename = convert_and_check_filename(PG_GETARG_TEXT_P(0), false); data = PG_GETARG_TEXT_P(1); - if (PG_ARGISNULL(2) || !PG_GETARG_BOOL(2)) + if (!PG_GETARG_BOOL(2)) { struct stat fst; if (stat(filename, &fst) >= 0) ereport(ERROR, (ERRCODE_DUPLICATE_FILE, - errmsg("file %s exists", filename))); + errmsg("file \"%s\" exists", filename))); f = fopen(filename, "wb"); } @@ -147,11 +151,10 @@ pg_file_write(PG_FUNCTION_ARGS) f = fopen(filename, "ab"); if (!f) - { ereport(ERROR, (errcode_for_file_access(), - errmsg("could open file %s for writing: %m", filename))); - } + errmsg("could not open file \"%s\" for writing: %m", + filename))); if (VARSIZE(data) != 0) { @@ -160,7 +163,7 @@ pg_file_write(PG_FUNCTION_ARGS) if (count != VARSIZE(data) - VARHDRSZ) ereport(ERROR, (errcode_for_file_access(), - errmsg("error writing file %s: %m", filename))); + errmsg("could not write file \"%s\": %m", filename))); } fclose(f); @@ -181,18 +184,18 @@ pg_file_rename(PG_FUNCTION_ARGS) if (PG_ARGISNULL(0) || PG_ARGISNULL(1)) PG_RETURN_NULL(); - fn1 = absClusterPath(PG_GETARG_TEXT_P(0), false); - fn2 = absClusterPath(PG_GETARG_TEXT_P(1), false); + fn1 = convert_and_check_filename(PG_GETARG_TEXT_P(0), false); + fn2 = convert_and_check_filename(PG_GETARG_TEXT_P(1), false); if (PG_ARGISNULL(2)) fn3 = 0; else - fn3 = absClusterPath(PG_GETARG_TEXT_P(2), false); + fn3 = convert_and_check_filename(PG_GETARG_TEXT_P(2), false); if (access(fn1, W_OK) < 0) { ereport(WARNING, (errcode_for_file_access(), - errmsg("file %s not accessible: %m", fn1))); + errmsg("file \"%s\" is not accessible: %m", fn1))); PG_RETURN_BOOL(false); } @@ -201,18 +204,18 @@ pg_file_rename(PG_FUNCTION_ARGS) { ereport(WARNING, (errcode_for_file_access(), - errmsg("file %s not accessible: %m", fn2))); + errmsg("file \"%s\" is not accessible: %m", fn2))); PG_RETURN_BOOL(false); } - rc = access(fn3 ? fn3 : fn2, 2); if (rc >= 0 || errno != ENOENT) { ereport(ERROR, (ERRCODE_DUPLICATE_FILE, - errmsg("cannot rename to target file %s", fn3 ? fn3 : fn2))); + errmsg("cannot rename to target file \"%s\"", + fn3 ? fn3 : fn2))); } if (fn3) @@ -221,37 +224,37 @@ pg_file_rename(PG_FUNCTION_ARGS) { ereport(ERROR, (errcode_for_file_access(), - errmsg("could not rename %s to %s: %m", fn2, fn3))); + errmsg("could not rename \"%s\" to \"%s\": %m", + fn2, fn3))); } if (rename(fn1, fn2) != 0) { ereport(WARNING, (errcode_for_file_access(), - errmsg("could not rename %s to %s: %m", fn1, fn2))); + errmsg("could not rename \"%s\" to \"%s\": %m", + fn1, fn2))); if (rename(fn3, fn2) != 0) { ereport(ERROR, (errcode_for_file_access(), - errmsg("could not rename %s back to %s: %m", fn3, fn2))); + errmsg("could not rename \"%s\" back to \"%s\": %m", + fn3, fn2))); } else { ereport(ERROR, (ERRCODE_UNDEFINED_FILE, - errmsg("renaming %s to %s was reverted", fn2, fn3))); - + errmsg("renaming \"%s\" to \"%s\" was reverted", + fn2, fn3))); } } } else if (rename(fn1, fn2) != 0) { - ereport(WARNING, - (errcode_for_file_access(), - errmsg("renaming %s to %s %m", fn1, fn2))); ereport(ERROR, (errcode_for_file_access(), - errmsg("could not rename %s to %s: %m", fn1, fn2))); + errmsg("could not rename \"%s\" to \"%s\": %m", fn1, fn2))); } PG_RETURN_BOOL(true); @@ -265,7 +268,7 @@ pg_file_unlink(PG_FUNCTION_ARGS) requireSuperuser(); - filename = absClusterPath(PG_GETARG_TEXT_P(0), false); + filename = convert_and_check_filename(PG_GETARG_TEXT_P(0), false); if (access(filename, W_OK) < 0) { @@ -274,15 +277,14 @@ pg_file_unlink(PG_FUNCTION_ARGS) else ereport(ERROR, (errcode_for_file_access(), - errmsg("file %s not accessible: %m", filename))); - + errmsg("file \"%s\" is not accessible: %m", filename))); } if (unlink(filename) < 0) { ereport(WARNING, (errcode_for_file_access(), - errmsg("could not unlink file %s: %m", filename))); + errmsg("could not unlink file \"%s\": %m", filename))); PG_RETURN_BOOL(false); } @@ -316,13 +318,7 @@ pg_logdir_ls(PG_FUNCTION_ARGS) oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); fctx = palloc(sizeof(directory_fctx)); - if (is_absolute_path(Log_directory)) - fctx->location = pstrdup(Log_directory); - else - { - fctx->location = palloc(strlen(DataDir) + strlen(Log_directory) + 2); - sprintf(fctx->location, "%s/%s", DataDir, Log_directory); - } + tupdesc = CreateTemplateTupleDesc(2, false); TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime", TIMESTAMPOID, -1, 0); @@ -331,12 +327,14 @@ pg_logdir_ls(PG_FUNCTION_ARGS) funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc); + fctx->location = pstrdup(Log_directory); fctx->dirdesc = AllocateDir(fctx->location); if (!fctx->dirdesc) ereport(ERROR, (errcode_for_file_access(), - errmsg("%s is not browsable: %m", fctx->location))); + errmsg("could not read directory \"%s\": %m", + fctx->location))); funcctx->user_fctx = fctx; MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 09c1be6b07..fa8b13787e 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -5,7 +5,7 @@ * Copyright (c) 2002-2006, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.8 2006/03/05 15:58:41 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.9 2006/11/06 03:06:41 tgl Exp $ * */ @@ -15,6 +15,7 @@ #include #include "access/heapam.h" +#include "catalog/catalog.h" #include "catalog/namespace.h" #include "catalog/pg_tablespace.h" #include "commands/dbcommands.h" @@ -77,11 +78,11 @@ calculate_database_size(Oid dbOid) /* Shared storage in pg_global is not counted */ /* Include pg_default storage */ - snprintf(pathname, MAXPGPATH, "%s/base/%u", DataDir, dbOid); + snprintf(pathname, MAXPGPATH, "base/%u", dbOid); totalsize = db_dir_size(pathname); /* Scan the non-default tablespaces */ - snprintf(dirpath, MAXPGPATH, "%s/pg_tblspc", DataDir); + snprintf(dirpath, MAXPGPATH, "pg_tblspc"); dirdesc = AllocateDir(dirpath); if (!dirdesc) ereport(ERROR, @@ -95,8 +96,8 @@ calculate_database_size(Oid dbOid) strcmp(direntry->d_name, "..") == 0) continue; - snprintf(pathname, MAXPGPATH, "%s/pg_tblspc/%s/%u", - DataDir, direntry->d_name, dbOid); + snprintf(pathname, MAXPGPATH, "pg_tblspc/%s/%u", + direntry->d_name, dbOid); totalsize += db_dir_size(pathname); } @@ -148,11 +149,11 @@ calculate_tablespace_size(Oid tblspcOid) struct dirent *direntry; if (tblspcOid == DEFAULTTABLESPACE_OID) - snprintf(tblspcPath, MAXPGPATH, "%s/base", DataDir); + snprintf(tblspcPath, MAXPGPATH, "base"); else if (tblspcOid == GLOBALTABLESPACE_OID) - snprintf(tblspcPath, MAXPGPATH, "%s/global", DataDir); + snprintf(tblspcPath, MAXPGPATH, "global"); else - snprintf(tblspcPath, MAXPGPATH, "%s/pg_tblspc/%u", DataDir, tblspcOid); + snprintf(tblspcPath, MAXPGPATH, "pg_tblspc/%u", tblspcOid); dirdesc = AllocateDir(tblspcPath); @@ -219,30 +220,22 @@ static int64 calculate_relation_size(RelFileNode *rfn) { int64 totalsize = 0; - char dirpath[MAXPGPATH]; + char *relationpath; char pathname[MAXPGPATH]; unsigned int segcount = 0; - Assert(OidIsValid(rfn->spcNode)); - - if (rfn->spcNode == DEFAULTTABLESPACE_OID) - snprintf(dirpath, MAXPGPATH, "%s/base/%u", DataDir, rfn->dbNode); - else if (rfn->spcNode == GLOBALTABLESPACE_OID) - snprintf(dirpath, MAXPGPATH, "%s/global", DataDir); - else - snprintf(dirpath, MAXPGPATH, "%s/pg_tblspc/%u/%u", - DataDir, rfn->spcNode, rfn->dbNode); + relationpath = relpath(*rfn); for (segcount = 0;; segcount++) { struct stat fst; if (segcount == 0) - snprintf(pathname, MAXPGPATH, "%s/%u", - dirpath, rfn->relNode); + snprintf(pathname, MAXPGPATH, "%s", + relationpath); else - snprintf(pathname, MAXPGPATH, "%s/%u.%u", - dirpath, rfn->relNode, segcount); + snprintf(pathname, MAXPGPATH, "%s.%u", + relationpath, segcount); if (stat(pathname, &fst) < 0) { @@ -296,8 +289,7 @@ pg_relation_size_name(PG_FUNCTION_ARGS) /* * Compute the on-disk size of files for the relation according to the - * stat function, optionally including heap data, index data, and/or - * toast data. + * stat function, including heap data, index data, and toast data. */ static int64 calculate_total_relation_size(Oid Relid) @@ -313,10 +305,9 @@ calculate_total_relation_size(Oid Relid) /* Get the heap size */ size = calculate_relation_size(&(heapRel->rd_node)); - /* Get index size */ + /* Include any dependent indexes */ if (heapRel->rd_rel->relhasindex) { - /* recursively include any dependent indexes */ List *index_oids = RelationGetIndexList(heapRel); foreach(cell, index_oids) @@ -334,7 +325,7 @@ calculate_total_relation_size(Oid Relid) list_free(index_oids); } - /* Get toast table (and index) size */ + /* Recursively include toast table (and index) size */ if (OidIsValid(toastOid)) size += calculate_total_relation_size(toastOid); @@ -343,10 +334,6 @@ calculate_total_relation_size(Oid Relid) return size; } -/* - * Compute on-disk size of files for 'relation' including - * heap data, index data, and toasted data. - */ Datum pg_total_relation_size_oid(PG_FUNCTION_ARGS) { diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 19a03cee0a..cee910f854 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -9,7 +9,7 @@ * Author: Andreas Pflug * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.11 2006/07/13 16:49:16 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.12 2006/11/06 03:06:41 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,13 +38,13 @@ typedef struct /* - * Validate a path and convert to absolute form. + * Convert a "text" filename argument to C string, and check it's allowable. * - * Argument may be absolute or relative to the DataDir (but we only allow - * absolute paths that match DataDir or Log_directory). + * Filename may be absolute or relative to the DataDir, but we only allow + * absolute paths that match DataDir or Log_directory. */ static char * -check_and_make_absolute(text *arg) +convert_and_check_filename(text *arg) { int input_len = VARSIZE(arg) - VARHDRSZ; char *filename = palloc(input_len + 1); @@ -77,11 +77,7 @@ check_and_make_absolute(text *arg) } else { - char *absname = palloc(strlen(DataDir) + strlen(filename) + 2); - - sprintf(absname, "%s/%s", DataDir, filename); - pfree(filename); - return absname; + return filename; } } @@ -105,7 +101,7 @@ pg_read_file(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("must be superuser to read files")))); - filename = check_and_make_absolute(filename_t); + filename = convert_and_check_filename(filename_t); if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL) ereport(ERROR, @@ -166,7 +162,7 @@ pg_stat_file(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("must be superuser to get file information")))); - filename = check_and_make_absolute(filename_t); + filename = convert_and_check_filename(filename_t); if (stat(filename, &fst) < 0) ereport(ERROR, @@ -238,7 +234,7 @@ pg_ls_dir(PG_FUNCTION_ARGS) oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); fctx = palloc(sizeof(directory_fctx)); - fctx->location = check_and_make_absolute(PG_GETARG_TEXT_P(0)); + fctx->location = convert_and_check_filename(PG_GETARG_TEXT_P(0)); fctx->dirdesc = AllocateDir(fctx->location);