From 402d42283b872b1abd1038a9e640a67f8a3248b9 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 2 Nov 2022 15:45:41 +0900 Subject: [PATCH v15 2/2] Some simplifications from me --- src/include/utils/conffiles.h | 23 ++++ src/include/utils/guc.h | 2 - src/backend/libpq/hba.c | 66 ++++++------ src/backend/utils/misc/Makefile | 1 + src/backend/utils/misc/conffiles.c | 161 ++++++++++++++++++++++++++++ src/backend/utils/misc/guc-file.l | 138 +----------------------- src/backend/utils/misc/meson.build | 1 + src/test/authentication/meson.build | 1 + 8 files changed, 221 insertions(+), 172 deletions(-) create mode 100644 src/include/utils/conffiles.h create mode 100644 src/backend/utils/misc/conffiles.c diff --git a/src/include/utils/conffiles.h b/src/include/utils/conffiles.h new file mode 100644 index 0000000000..3f23a2a011 --- /dev/null +++ b/src/include/utils/conffiles.h @@ -0,0 +1,23 @@ +/*-------------------------------------------------------------------- + * conffiles.h + * + * Utilities related to configuration files. + * + * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/utils/conffiles.h + * + *-------------------------------------------------------------------- + */ +#ifndef CONFFILES_H +#define CONFFILES_H + +extern char *AbsoluteConfigLocation(const char *location, + const char *calling_file); +extern char **GetConfFilesInDir(const char *includedir, + const char *calling_file, + int elevel, int *num_filenames, + char **err_msg); + +#endif /* CONFFILES_H */ diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 59ca39d908..b3aaff9665 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -144,8 +144,6 @@ typedef struct ConfigVariable struct ConfigVariable *next; } ConfigVariable; -extern char **GetDirConfFiles(const char *includedir, const char *calling_file, - int elevel, int *num_filenames, char **err_msg); extern bool ParseConfigFile(const char *config_file, bool strict, const char *calling_file, int calling_lineno, int depth, int elevel, diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 1034ff8220..7ebbd5d7b1 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -42,6 +41,7 @@ #include "storage/fd.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/conffiles.h" #include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -122,10 +122,9 @@ static const char *const UserAuthName[] = }; -static void tokenize_file_with_context(MemoryContext linecxt, - const char *filename, FILE *file, - List **tok_lines, int depth, - int elevel); +static void tokenize_auth_file_internal(const char *filename, FILE *file, + List **tok_lines, int depth, + int elevel); static List *tokenize_inc_file(List *tokens, const char *outer_filename, const char *inc_filename, int depth, int elevel, char **err_msg); @@ -138,9 +137,9 @@ static int regexec_auth_token(const char *match, AuthToken *token, static FILE *open_inc_file(HbaIncludeKind kind, const char *inc_filename, bool strict, const char *outer_filename, int elevel, char **err_msg, char **inc_fullname); -static char *process_included_authfile(const char *inc_filename, bool strict, +static char *process_included_auth_file(const char *inc_filename, bool strict, const char *outer_filename, int depth, - int elevel, MemoryContext linecxt, + int elevel, List **tok_lines); @@ -508,8 +507,8 @@ tokenize_inc_file(List *tokens, return tokens; /* There is possible recursion here if the file contains @ */ - linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, depth + 1, - elevel); + linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, + depth + 1, elevel); FreeFile(inc_file); pfree(inc_fullname); @@ -548,8 +547,9 @@ tokenize_inc_file(List *tokens, /* * tokenize_auth_file * - * Wrapper around tokenize_file_with_context, creating a dedicated memory - * context. + * Wrapper around tokenize_auth_file_internal, creating a dedicated memory + * context where all the magic happens, working as an entry point for + * the tokenization of the HBA and ident files. * * Return value is this memory context which contains all memory allocated by * this function (it's a child of caller's context). @@ -559,14 +559,19 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, int depth, int elevel) { MemoryContext linecxt; + MemoryContext oldcxt; + linecxt = AllocSetContextCreate(CurrentMemoryContext, "tokenize_auth_file", ALLOCSET_SMALL_SIZES); + oldcxt = MemoryContextSwitchTo(linecxt); *tok_lines = NIL; - tokenize_file_with_context(linecxt, filename, file, tok_lines, depth, - elevel); + /* can recurse into itself */ + tokenize_auth_file_internal(filename, file, tok_lines, depth, elevel); + + MemoryContextSwitchTo(oldcxt); return linecxt; } @@ -577,8 +582,6 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, * The output is a list of TokenizedAuthLine structs; see the struct definition * in libpq/hba.h. * - * linecxt: memory context which must contain all memory allocated by the - * function * filename: the absolute path to the target file * file: the already-opened target file * tok_lines: receives output list @@ -589,14 +592,11 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, * output list. */ static void -tokenize_file_with_context(MemoryContext linecxt, const char *filename, - FILE *file, List **tok_lines, int depth, int elevel) +tokenize_auth_file_internal(const char *filename, + FILE *file, List **tok_lines, int depth, int elevel) { StringInfoData buf; int line_number = 1; - MemoryContext oldcxt; - - oldcxt = MemoryContextSwitchTo(linecxt); initStringInfo(&buf); @@ -683,8 +683,8 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename, inc_filename = second->string; - err_msg = process_included_authfile(inc_filename, true, - filename, depth + 1, elevel, linecxt, + err_msg = process_included_auth_file(inc_filename, true, + filename, depth + 1, elevel, tok_lines); if (!err_msg) @@ -703,7 +703,7 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename, int num_filenames; StringInfoData err_buf; - filenames = GetDirConfFiles(dir_name, filename, elevel, + filenames = GetConfFilesInDir(dir_name, filename, elevel, &num_filenames, &err_msg); if (!filenames) @@ -720,9 +720,9 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename, * overwritten at the end of the loop with the * cumulated errors, if any. */ - err_msg = process_included_authfile(filenames[i], true, + err_msg = process_included_auth_file(filenames[i], true, filename, depth + 1, elevel, - linecxt, tok_lines); + tok_lines); /* Cumulate errors if any. */ if (err_msg) @@ -749,8 +749,8 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename, inc_filename = second->string; - err_msg = process_included_authfile(inc_filename, false, - filename, depth + 1, elevel, linecxt, + err_msg = process_included_auth_file(inc_filename, false, + filename, depth + 1, elevel, tok_lines); if (!err_msg) @@ -780,8 +780,6 @@ process_line: next_line: line_number += continuations + 1; } - - MemoryContextSwitchTo(oldcxt); } /* @@ -2650,9 +2648,9 @@ open_inc_file(HbaIncludeKind kind, const char *inc_filename, bool strict, * Returns NULL if no error happens during tokenization, otherwise the error. */ static char * -process_included_authfile(const char *inc_filename, bool strict, - const char *outer_filename, int depth, int elevel, - MemoryContext linecxt, List **tok_lines) +process_included_auth_file(const char *inc_filename, bool strict, + const char *outer_filename, int depth, int elevel, + List **tok_lines) { char *inc_fullname; FILE *inc_file; @@ -2693,8 +2691,8 @@ process_included_authfile(const char *inc_filename, bool strict, Assert(err_msg == NULL); } - tokenize_file_with_context(linecxt, inc_fullname, inc_file, - tok_lines, depth, elevel); + tokenize_auth_file_internal(inc_fullname, inc_file, + tok_lines, depth, elevel); FreeFile(inc_file); pfree(inc_fullname); diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile index 6097309033..b9ee4eb48a 100644 --- a/src/backend/utils/misc/Makefile +++ b/src/backend/utils/misc/Makefile @@ -15,6 +15,7 @@ include $(top_builddir)/src/Makefile.global override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS) OBJS = \ + conffiles.o \ guc.o \ guc-file.o \ guc_funcs.o \ diff --git a/src/backend/utils/misc/conffiles.c b/src/backend/utils/misc/conffiles.c new file mode 100644 index 0000000000..c9659a7c70 --- /dev/null +++ b/src/backend/utils/misc/conffiles.c @@ -0,0 +1,161 @@ +/*-------------------------------------------------------------------- + * conffiles.c + * + * Utilities and tools related to the handling of configuration files. + * + * This file contains the generic tools to work on configuration files + * used by PostgreSQL, be they related to GUC, HBA or ident files. + * + * + * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/backend/utils/misc/conffiles.c + * + *-------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include + +#include "common/file_utils.h" +#include "miscadmin.h" +#include "storage/fd.h" +#include "utils/conffiles.h" + +/* + * Given a configuration file or directory location that may be a relative + * path, return an absolute one. We consider the location to be relative to + * the directory holding the calling file, or to DataDir if no calling file. + */ +char * +AbsoluteConfigLocation(const char *location, const char *calling_file) +{ + char abs_path[MAXPGPATH]; + + if (is_absolute_path(location)) + return pstrdup(location); + else + { + if (calling_file != NULL) + { + strlcpy(abs_path, calling_file, sizeof(abs_path)); + get_parent_directory(abs_path); + join_path_components(abs_path, abs_path, location); + canonicalize_path(abs_path); + } + else + { + Assert(DataDir); + join_path_components(abs_path, DataDir, location); + canonicalize_path(abs_path); + } + return pstrdup(abs_path); + } +} + + +/* + * Returns the list of config files located in a directory, in alphabetical + * order. + * + * On error, returns NULL with details stored in "err_msg". + */ +char ** +GetConfFilesInDir(const char *includedir, const char *calling_file, + int elevel, int *num_filenames, char **err_msg) +{ + char *directory; + DIR *d; + struct dirent *de; + char **filenames = NULL; + int size_filenames; + + /* + * Reject directory name that is all-blank (including empty), as that + * leads to confusion --- we'd read the containing directory, typically + * resulting in recursive inclusion of the same file(s). + */ + if (strspn(includedir, " \t\r\n") == strlen(includedir)) + { + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("empty configuration directory name: \"%s\"", + includedir))); + *err_msg = "empty configuration directory name"; + return NULL; + } + + directory = AbsoluteConfigLocation(includedir, calling_file); + d = AllocateDir(directory); + if (d == NULL) + { + ereport(elevel, + (errcode_for_file_access(), + errmsg("could not open configuration directory \"%s\": %m", + directory))); + *err_msg = psprintf("could not open directory \"%s\"", directory); + goto cleanup; + } + + /* + * Read the directory and put the filenames in an array, so we can sort + * them prior to caller processing the contents. + */ + size_filenames = 32; + filenames = (char **) palloc(size_filenames * sizeof(char *)); + *num_filenames = 0; + + while ((de = ReadDir(d, directory)) != NULL) + { + PGFileType de_type; + char filename[MAXPGPATH]; + + /* + * Only parse files with names ending in ".conf". Explicitly reject + * files starting with ".". This excludes things like "." and "..", + * as well as typical hidden files, backup files, and editor debris. + */ + if (strlen(de->d_name) < 6) + continue; + if (de->d_name[0] == '.') + continue; + if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0) + continue; + + join_path_components(filename, directory, de->d_name); + canonicalize_path(filename); + de_type = get_dirent_type(filename, de, true, elevel); + if (de_type == PGFILETYPE_ERROR) + { + *err_msg = psprintf("could not stat file \"%s\"", filename); + pfree(filenames); + filenames = NULL; + goto cleanup; + } + else if (de_type != PGFILETYPE_DIR) + { + /* Add file to array, increasing its size in blocks of 32 */ + if (*num_filenames >= size_filenames) + { + size_filenames += 32; + filenames = (char **) repalloc(filenames, + size_filenames * sizeof(char *)); + } + filenames[*num_filenames] = pstrdup(filename); + (*num_filenames)++; + } + } + + /* Sort the files by name before leaving */ + if (*num_filenames > 0) + qsort(filenames, *num_filenames, sizeof(char *), pg_qsort_strcmp); + +cleanup: + if (d) + FreeDir(d); + pfree(directory); + return filenames; +} diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index f76a56398b..937dc5fbf8 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -18,6 +18,7 @@ #include "miscadmin.h" #include "storage/fd.h" #include +#include "utils/conffiles.h" #include "utils/memutils.h" } @@ -156,37 +157,6 @@ ProcessConfigFile(GucContext context) MemoryContextDelete(config_cxt); } -/* - * Given a configuration file or directory location that may be a relative - * path, return an absolute one. We consider the location to be relative to - * the directory holding the calling file, or to DataDir if no calling file. - */ -static char * -AbsoluteConfigLocation(const char *location, const char *calling_file) -{ - char abs_path[MAXPGPATH]; - - if (is_absolute_path(location)) - return pstrdup(location); - else - { - if (calling_file != NULL) - { - strlcpy(abs_path, calling_file, sizeof(abs_path)); - get_parent_directory(abs_path); - join_path_components(abs_path, abs_path, location); - canonicalize_path(abs_path); - } - else - { - Assert(DataDir); - join_path_components(abs_path, DataDir, location); - canonicalize_path(abs_path); - } - return pstrdup(abs_path); - } -} - /* * Read and parse a single configuration file. This function recurses * to handle "include" directives. @@ -345,110 +315,6 @@ GUC_flex_fatal(const char *msg) return 0; /* keep compiler quiet */ } -/* - * Returns the list of config files located in a directory, in alphabetical - * order. - * - * We don't check for recursion or too-deep nesting depth here, its up to the - * caller to take care of that. - */ -char ** -GetDirConfFiles(const char *includedir, const char *calling_file, int elevel, - int *num_filenames, char **err_msg) -{ - char *directory; - DIR *d; - struct dirent *de; - char **filenames; - int size_filenames; - - /* - * Reject directory name that is all-blank (including empty), as that - * leads to confusion --- we'd read the containing directory, typically - * resulting in recursive inclusion of the same file(s). - */ - if (strspn(includedir, " \t\r\n") == strlen(includedir)) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("empty configuration directory name: \"%s\"", - includedir))); - *err_msg = "empty configuration directory name"; - return NULL; - } - - directory = AbsoluteConfigLocation(includedir, calling_file); - d = AllocateDir(directory); - if (d == NULL) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not open configuration directory \"%s\": %m", - directory))); - *err_msg = psprintf("could not open directory \"%s\"", directory); - filenames = NULL; - goto cleanup; - } - - /* - * Read the directory and put the filenames in an array, so we can sort - * them prior to caller processing the contents. - */ - size_filenames = 32; - filenames = (char **) palloc(size_filenames * sizeof(char *)); - *num_filenames = 0; - - while ((de = ReadDir(d, directory)) != NULL) - { - PGFileType de_type; - char filename[MAXPGPATH]; - - /* - * Only parse files with names ending in ".conf". Explicitly reject - * files starting with ".". This excludes things like "." and "..", - * as well as typical hidden files, backup files, and editor debris. - */ - if (strlen(de->d_name) < 6) - continue; - if (de->d_name[0] == '.') - continue; - if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0) - continue; - - join_path_components(filename, directory, de->d_name); - canonicalize_path(filename); - de_type = get_dirent_type(filename, de, true, elevel); - if (de_type == PGFILETYPE_ERROR) - { - *err_msg = psprintf("could not stat file \"%s\"", filename); - pfree(filenames); - filenames = NULL; - goto cleanup; - } - else if (de_type != PGFILETYPE_DIR) - { - /* Add file to array, increasing its size in blocks of 32 */ - if (*num_filenames >= size_filenames) - { - size_filenames += 32; - filenames = (char **) repalloc(filenames, - size_filenames * sizeof(char *)); - } - filenames[*num_filenames] = pstrdup(filename); - (*num_filenames)++; - } - } - - if (*num_filenames > 0) - qsort(filenames, *num_filenames, sizeof(char *), pg_qsort_strcmp); - -cleanup: - if (d) - FreeDir(d); - pfree(directory); - return filenames; -} - /* * Read and parse a single configuration file. This function recurses * to handle "include" directives. @@ -714,7 +580,7 @@ ParseConfigDirectory(const char *includedir, char **filenames; int num_filenames; - filenames = GetDirConfFiles(includedir, calling_file, elevel, + filenames = GetConfFilesInDir(includedir, calling_file, elevel, &num_filenames, &err_msg); if (!filenames) diff --git a/src/backend/utils/misc/meson.build b/src/backend/utils/misc/meson.build index db4de225e1..e7a9730229 100644 --- a/src/backend/utils/misc/meson.build +++ b/src/backend/utils/misc/meson.build @@ -1,4 +1,5 @@ backend_sources += files( + 'conffiles.c', 'guc.c', 'guc_funcs.c', 'guc_tables.c', diff --git a/src/test/authentication/meson.build b/src/test/authentication/meson.build index c2b48c43c9..cfc23fa213 100644 --- a/src/test/authentication/meson.build +++ b/src/test/authentication/meson.build @@ -7,6 +7,7 @@ tests += { 't/001_password.pl', 't/002_saslprep.pl', 't/003_peer.pl', + 't/004_file_inclusion.pl', ], }, } -- 2.38.1