Skip to content

Commit

Permalink
Modify APIs to return palloc'ed values and catalog lookup to validate…
Browse files Browse the repository at this point in the history 
… the user in get_physical_user_name (babelfish-for-postgresql#2960)

Earlier in Babelfish, The get_*_role_name() and get_*_schema_name() APIs returned constant values or string literal for
few cases and palloc'ed copy for other cases. The get_physical_user_name() used to return the user_name after
appending the db_name to the user_name without any checks to verify its existence/validity.

With this change, We return Palloc'ed values for the API every time. In get_physical_user_name() if the user or role is
not found in the sys.babelfish_authid_user_ext catalog, then an error is thrown. Additionally, In multidb.c, made
changes to the APIs like get_role_name() and get_schema_name() to return palloc'ed values instead of constant
values. 

Implemented a catalog lookup in get_physical_user_name() to verify if the user or role is found in the
sys.babelfish_authid_user_ext catalog. The 'suppress_role_error' flag indicates if it is ok for the user or role to be
absent from the catalog (required for cases when a new user/role is created and it can't be found in the catalog).

Checked all the functions where the APIs are called and pfree the variables containing values from the above APIs after they are no longer used.

Task: BABEL-4933
Signed-off-by: P Aswini Kumar <[email protected]>
  • Loading branch information
@aswiniip
aswiniip authored Sep 30, 2024
1 parent 2b0b6d5 commit 1550114
Show file tree
Hide file tree
Showing 14 changed files with 324 additions and 205 deletions.
37 changes: 26 additions & 11 deletions contrib/babelfishpg_tsql/runtime/functions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1303,11 +1303,13 @@ schema_id(PG_FUNCTION_ARGS)
{
char *db_name = get_cur_db_name();
const char *user = get_user_for_database(db_name);
const char *guest_role_name = get_guest_role_name(db_name);
char *guest_role_name = get_guest_role_name(db_name);

if (!user)
{
pfree(db_name);
pfree(guest_role_name);

PG_RETURN_NULL();
}
else if ((guest_role_name && strcmp(user, guest_role_name) == 0))
Expand All @@ -1320,6 +1322,7 @@ schema_id(PG_FUNCTION_ARGS)
physical_name = get_physical_schema_name(db_name, name);
}
pfree(db_name);
pfree(guest_role_name);
}
else
{
Expand Down Expand Up @@ -2224,15 +2227,18 @@ object_id(PG_FUNCTION_ARGS)
* name
*/
const char *user = get_user_for_database(db_name);
const char *guest_role_name = get_guest_role_name(db_name);
char *guest_role_name = get_guest_role_name(db_name);

if (!user)
{
pfree(db_name);
pfree(schema_name);
pfree(object_name);
pfree(guest_role_name);

if (object_type)
pfree(object_type);

PG_RETURN_NULL();
}
else if ((guest_role_name && strcmp(user, guest_role_name) == 0))
Expand All @@ -2245,6 +2251,8 @@ object_id(PG_FUNCTION_ARGS)
schema_name = get_authid_user_ext_schema_name((const char *) db_name, user);
physical_schema_name = get_physical_schema_name(db_name, schema_name);
}

pfree(guest_role_name);
}
else
{
Expand Down Expand Up @@ -2677,14 +2685,16 @@ type_id(PG_FUNCTION_ARGS)
if (!OidIsValid(result))
{
/* find the default schema for current user and get physical schema name */
const char *user = get_user_for_database(db_name);
const char *guest_role_name = get_guest_role_name(db_name);
const char *user = get_user_for_database(db_name);
char *guest_role_name = get_guest_role_name(db_name);

if (!user)
{
pfree(db_name);
pfree(schema_name);
pfree(object_name);
pfree(guest_role_name);

PG_RETURN_NULL();
}
else if ((guest_role_name && strcmp(user, guest_role_name) == 0))
Expand All @@ -2697,6 +2707,8 @@ type_id(PG_FUNCTION_ARGS)
schema_name = get_authid_user_ext_schema_name((const char *) db_name, user);
physical_schema_name = get_physical_schema_name(db_name, schema_name);
}

pfree(guest_role_name);
}
else
{
Expand Down Expand Up @@ -2826,20 +2838,20 @@ replace_special_chars_fts(PG_FUNCTION_ARGS)
Datum
has_dbaccess(PG_FUNCTION_ARGS)
{
char *db_name = text_to_cstring(PG_GETARG_TEXT_P(0));
char *db_name = text_to_cstring(PG_GETARG_TEXT_P(0));

/*
* Ensure the database name input argument is lower-case, as all Babel
* table names are lower-case
*/
char *lowercase_db_name = lowerstr(db_name);
char *lowercase_db_name = lowerstr(db_name);

/* Also strip trailing whitespace to mimic SQL Server behaviour */
int i;
const char *user = NULL;
const char *login;
int16 db_id;
bool login_is_db_owner;
int i;
char *user = NULL;
const char *login;
int16 db_id;
bool login_is_db_owner;

i = strlen(lowercase_db_name);
while (i > 0 && isspace((unsigned char) lowercase_db_name[i - 1]))
Expand Down Expand Up @@ -2885,7 +2897,10 @@ has_dbaccess(PG_FUNCTION_ARGS)
if (!user)
PG_RETURN_INT32(0);
else
{
pfree(user);
PG_RETURN_INT32(1);
}
}

Datum
Expand Down
49 changes: 44 additions & 5 deletions contrib/babelfishpg_tsql/src/catalog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3041,7 +3041,7 @@ update_user_catalog_for_guest(PG_FUNCTION_ARGS)
bool
guest_role_exists_for_db(const char *dbname)
{
const char *guest_role = get_guest_role_name(dbname);
char *guest_role = get_guest_role_name(dbname);
bool role_exists = false;
HeapTuple tuple;

Expand All @@ -3053,6 +3053,8 @@ guest_role_exists_for_db(const char *dbname)
ReleaseSysCache(tuple);
}

pfree(guest_role);

return role_exists;
}

Expand Down Expand Up @@ -3109,7 +3111,7 @@ get_login_for_user(Oid user_id, const char *physical_schema_name)
static void
create_guest_role_for_db(const char *dbname)
{
const char *guest = get_guest_role_name(dbname);
char *guest = get_guest_role_name(dbname);
const char *db_owner_role = get_db_owner_role_name(dbname);
List *logins = NIL;
List *res;
Expand Down Expand Up @@ -3195,6 +3197,8 @@ create_guest_role_for_db(const char *dbname)
set_cur_db(old_dbid, old_dbname);
}
PG_END_TRY();

pfree(guest);
}

/*
Expand Down Expand Up @@ -4659,7 +4663,7 @@ update_babelfish_authid_user_ext_rename_db(
Anum_bbf_authid_user_ext_orig_username, bbf_authid_user_ext_dsc, &isNull));
NameData rolename_namedata;

namestrcpy(&rolename_namedata, get_physical_user_name((char *)new_db_name, role_name, true));
namestrcpy(&rolename_namedata, get_physical_user_name((char *)new_db_name, role_name, true, true));
list_of_roles_to_rename = lappend(list_of_roles_to_rename, pstrdup(role_name));

/* update rolname */
Expand Down Expand Up @@ -4938,9 +4942,12 @@ rename_tsql_db(char *old_db_name, char *new_db_name)
(strlen(role) == 8 && strncmp(role, "db_owner", 8) == 0)))
continue;

old_role_name = get_physical_user_name(old_db_name, role, true);
new_role_name = get_physical_user_name(new_db_name, role, true);
old_role_name = get_physical_user_name(old_db_name, role, true, true);
new_role_name = get_physical_user_name(new_db_name, role, true, true);
exec_rename_db_util(old_role_name, new_role_name, false);

pfree(old_role_name);
pfree(new_role_name);
}

/* Update the default_database field in babelfish_authid_login_ext. */
Expand Down Expand Up @@ -4977,6 +4984,38 @@ rename_tsql_db(char *old_db_name, char *new_db_name)
CommitTransactionCommand();
}

/*
* Returns true if the user/role exists in the sys.babelfish_authid_user_ext catalog,
* false otherwise.
*/
bool
user_exists_for_db(const char *db_name, const char *user_name)
{
HeapTuple tuple_cache;
NameData rolname;
bool user_exists = false;

namestrcpy(&rolname, user_name);

tuple_cache = SearchSysCache1(AUTHIDUSEREXTROLENAME, NameGetDatum(&rolname));

if (HeapTupleIsValid(tuple_cache))
{
bool isnull;
char *db_name_from_cache = TextDatumGetCString(SysCacheGetAttr(AUTHIDUSEREXTROLENAME, tuple_cache,
Anum_bbf_authid_user_ext_database_name, &isnull));

Assert(!isnull);

if (strcmp(db_name_from_cache, db_name) == 0)
user_exists = true;

pfree(db_name_from_cache);
}

return user_exists;
}

/*
* partition_function_id_exists
* Returns true if provided function id is in use, false otherwise.
Expand Down
1 change: 1 addition & 0 deletions contrib/babelfishpg_tsql/src/catalog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ extern List *update_babelfish_namespace_ext_nsp_name(int16 db_id, char *new_db_n
extern List *update_babelfish_authid_user_ext_db_name(const char *old_db_name, const char *new_db_name);
extern void rename_tsql_db(char *old_db_name, char *new_db_name);
extern Oid get_login_for_user(Oid user_id, const char *physical_schema_name);
extern bool user_exists_for_db(const char *db_name, const char *user_name);

/* MUST comply with babelfish_authid_user_ext table */
typedef struct FormData_authid_user_ext
Expand Down
74 changes: 40 additions & 34 deletions contrib/babelfishpg_tsql/src/dbcmds.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,25 +450,25 @@ check_database_collation_name(const char *database_collation_name)
static void
create_bbf_db_internal(ParseState *pstate, const char *dbname, List *options, const char *owner, int16 dbid)
{
int16 old_dbid;
char *old_dbname;
Oid datdba;
Datum *new_record;
bool *new_record_nulls;
Relation sysdatabase_rel;
HeapTuple tuple;
List *parsetree_list;
ListCell *parsetree_item;
const char *dbo_role;
NameData default_collation;
NameData owner_namedata;
int stmt_number = 0;
int save_sec_context;
bool is_set_userid = false;
Oid save_userid;
const char *old_createrole_self_grant;
ListCell *option;
const char *database_collation_name = NULL;
int16 old_dbid;
char *old_dbname;
Oid datdba;
Datum *new_record;
bool *new_record_nulls;
Relation sysdatabase_rel;
HeapTuple tuple;
List *parsetree_list;
ListCell *parsetree_item;
char *dbo_role;
NameData default_collation;
NameData owner_namedata;
int stmt_number = 0;
int save_sec_context;
bool is_set_userid = false;
Oid save_userid;
const char *old_createrole_self_grant;
ListCell *option;
const char *database_collation_name = NULL;

/* Check options */
foreach(option, options)
Expand Down Expand Up @@ -636,23 +636,25 @@ create_bbf_db_internal(ParseState *pstate, const char *dbname, List *options, co
set_cur_db(old_dbid, old_dbname);
}
PG_END_TRY();

pfree(dbo_role);
}

void
drop_bbf_db(const char *dbname, bool missing_ok, bool force_drop)
{
volatile Relation sysdatabase_rel;
HeapTuple tuple;
Form_sysdatabases bbf_db;
int16 dbid;
const char *dbo_role;
List *db_users_list;
List *parsetree_list;
ListCell *parsetree_item;
const char *prev_current_user;
int save_sec_context;
bool is_set_userid = false;
Oid save_userid;
volatile Relation sysdatabase_rel;
HeapTuple tuple;
Form_sysdatabases bbf_db;
int16 dbid;
char *dbo_role;
List *db_users_list;
List *parsetree_list;
ListCell *parsetree_item;
const char *prev_current_user;
int save_sec_context;
bool is_set_userid = false;
Oid save_userid;

if ((strlen(dbname) == 6 && (strncmp(dbname, "master", 6) == 0)) ||
((strlen(dbname) == 6 && strncmp(dbname, "tempdb", 6) == 0)) ||
Expand Down Expand Up @@ -808,6 +810,8 @@ drop_bbf_db(const char *dbname, bool missing_ok, bool force_drop)
}
PG_END_TRY();

pfree(dbo_role);

/* Set current user back to previous user */
bbf_set_current_user(prev_current_user);
}
Expand Down Expand Up @@ -1064,8 +1068,8 @@ create_schema_if_not_exists(const uint16 dbid,
const char *prev_current_user;
uint16 old_dbid;
const char *old_dbname,
*phys_schema_name,
*phys_role;
*phys_schema_name;
char *phys_role;

/*
* During upgrade, the migration mode is reset to single-db so we cannot
Expand All @@ -1089,7 +1093,7 @@ create_schema_if_not_exists(const uint16 dbid,
* some reason guest role does not exist, then that is a bigger problem.
* We skip creating the guest schema entirely instead of crashing though.
*/
phys_role = get_physical_user_name((char *) dbname, (char *) owner_role, false);
phys_role = get_physical_user_name((char *) dbname, (char *) owner_role, false, true);
if (!OidIsValid(get_role_oid(phys_role, true)))
{
ereport(LOG,
Expand Down Expand Up @@ -1148,6 +1152,8 @@ create_schema_if_not_exists(const uint16 dbid,
}
PG_END_TRY();

pfree(phys_role);

bbf_set_current_user(prev_current_user);
set_cur_db(old_dbid, old_dbname);

Expand Down
7 changes: 5 additions & 2 deletions contrib/babelfishpg_tsql/src/hooks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5022,8 +5022,8 @@ get_local_schema_for_bbf_functions(Oid proc_nsp_oid)
HeapTuple tuple;
char *func_schema_name = NULL,
*new_search_path = NULL;
const char *func_dbo_schema,
*cur_dbname = get_cur_db_name();
char *func_dbo_schema;
const char *cur_dbname = get_cur_db_name();

tuple = SearchSysCache1(NAMESPACEOID,
ObjectIdGetDatum(proc_nsp_oid));
Expand All @@ -5039,7 +5039,10 @@ get_local_schema_for_bbf_functions(Oid proc_nsp_oid)
quote_identifier(func_dbo_schema));

ReleaseSysCache(tuple);

pfree(func_dbo_schema);
}

return new_search_path;
}

Expand Down
Loading

0 comments on commit 1550114

Please sign in to comment.