Skip to content

Commit 9b018bb

Browse files
sfc-gh-npukasfc-gh-npuka
committed
Block reserved catalog names and fix prefix-match comparisons
Block CREATE SERVER with a name that case-insensitively matches the extension-owned catalog names (postgres, object_store, rest) on the iceberg_catalog FDW. Also block ALTER SERVER ... RENAME TO a reserved name. Fix a latent bug across multiple files where pg_strncasecmp was used with a partial length, causing prefix-only matching: e.g. "rest_1" would match "rest". Replaced all instances with pg_strcasecmp for exact case-insensitive comparison. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
1 parent 5cc137e commit 9b018bb

5 files changed

Lines changed: 76 additions & 11 deletions

File tree

‎pg_lake_engine/src/utils/catalog_type.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ HasObjectStoreCatalogTableOption(List *options)
9090
{
9191
char *catalog = GetStringOption(options, "catalog", false);
9292

93-
return catalog ? pg_strncasecmp(catalog, OBJECT_STORE_CATALOG_NAME, strlen(catalog)) == 0 : false;
93+
return catalog ? pg_strcasecmp(catalog, OBJECT_STORE_CATALOG_NAME) == 0 : false;
9494
}
9595

9696

@@ -114,9 +114,9 @@ HasReadOnlyOption(List *options)
114114
bool
115115
IsCatalogOwnedByExtension(const char *catalog)
116116
{
117-
return pg_strncasecmp(catalog, REST_CATALOG_NAME, strlen(REST_CATALOG_NAME)) == 0 ||
118-
pg_strncasecmp(catalog, OBJECT_STORE_CATALOG_NAME, strlen(OBJECT_STORE_CATALOG_NAME)) == 0 ||
119-
pg_strncasecmp(catalog, POSTGRES_CATALOG_NAME, strlen(POSTGRES_CATALOG_NAME)) == 0;
117+
return pg_strcasecmp(catalog, REST_CATALOG_NAME) == 0 ||
118+
pg_strcasecmp(catalog, OBJECT_STORE_CATALOG_NAME) == 0 ||
119+
pg_strcasecmp(catalog, POSTGRES_CATALOG_NAME) == 0;
120120
}
121121

122122

@@ -131,7 +131,7 @@ IsRestCatalog(const char *catalog)
131131
if (catalog == NULL)
132132
return false;
133133

134-
if (pg_strncasecmp(catalog, REST_CATALOG_NAME, strlen(REST_CATALOG_NAME)) == 0)
134+
if (pg_strcasecmp(catalog, REST_CATALOG_NAME) == 0)
135135
return true;
136136

137137
/* Try to look up a server with this name */
@@ -148,7 +148,7 @@ IsRestCatalog(const char *catalog)
148148

149149
/* Check server TYPE if set */
150150
if (server->servertype != NULL && *server->servertype != '\0')
151-
return pg_strncasecmp(server->servertype, "rest", strlen("rest")) == 0;
151+
return pg_strcasecmp(server->servertype, "rest") == 0;
152152

153153
/* No TYPE specified, assume rest */
154154
return true;

‎pg_lake_iceberg/src/init.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ IcebergDefaultCatalogCheckHook(char **newvalue, void **extra, GucSource source)
367367
{
368368
char *newCatalog = *newvalue;
369369

370-
if (pg_strncasecmp(newCatalog, POSTGRES_CATALOG_NAME, strlen(newCatalog)) == 0 ||
371-
pg_strncasecmp(newCatalog, REST_CATALOG_NAME, strlen(newCatalog)) == 0 ||
372-
pg_strncasecmp(newCatalog, OBJECT_STORE_CATALOG_NAME, strlen(newCatalog)) == 0)
370+
if (pg_strcasecmp(newCatalog, POSTGRES_CATALOG_NAME) == 0 ||
371+
pg_strcasecmp(newCatalog, REST_CATALOG_NAME) == 0 ||
372+
pg_strcasecmp(newCatalog, OBJECT_STORE_CATALOG_NAME) == 0)
373373
return true;
374374

375375
GUC_check_errdetail("pg_lake_iceberg: allowed iceberg catalog options are '" POSTGRES_CATALOG_NAME "', "

‎pg_lake_iceberg/src/rest_catalog/rest_catalog.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,13 @@ BlockDDLOnExtensionCatalogs(ProcessUtilityParams *processUtilityParams,
230230
strcmp(stmt->fdwname, ICEBERG_CATALOG_FDW_NAME) != 0)
231231
return false;
232232

233+
if (IsCatalogOwnedByExtension(stmt->servername))
234+
ereport(ERROR,
235+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
236+
errmsg("server name \"%s\" is reserved for the extension-owned catalog",
237+
stmt->servername),
238+
errhint("Choose a different server name.")));
239+
233240
if (stmt->servertype != NULL &&
234241
(pg_strcasecmp(stmt->servertype, POSTGRES_CATALOG_NAME) == 0 ||
235242
pg_strcasecmp(stmt->servertype, OBJECT_STORE_CATALOG_NAME) == 0))
@@ -295,6 +302,13 @@ BlockDDLOnExtensionCatalogs(ProcessUtilityParams *processUtilityParams,
295302
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
296303
errmsg("cannot rename the extension-owned \"%s\" catalog server",
297304
serverName)));
305+
306+
if (IsCatalogOwnedByExtension(stmt->newname))
307+
ereport(ERROR,
308+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
309+
errmsg("server name \"%s\" is reserved for the extension-owned catalog",
310+
stmt->newname),
311+
errhint("Choose a different server name.")));
298312
}
299313
else if (IsA(parsetree, AlterOwnerStmt))
300314
{

‎pg_lake_table/src/fdw/option.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
767767
*/
768768
icebergCatalogType = REST_CATALOG_READ_ONLY;
769769
}
770-
else if (pg_strncasecmp(icebergCatalogName, OBJECT_STORE_CATALOG_NAME, strlen(icebergCatalogName)) == 0)
770+
else if (pg_strcasecmp(icebergCatalogName, OBJECT_STORE_CATALOG_NAME) == 0)
771771
{
772772
/*
773773
* at this point, we cannot tell whether it's read only or
@@ -776,7 +776,7 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
776776
*/
777777
icebergCatalogType = OBJECT_STORE_READ_ONLY;
778778
}
779-
else if (pg_strncasecmp(icebergCatalogName, POSTGRES_CATALOG_NAME, strlen(icebergCatalogName)) == 0)
779+
else if (pg_strcasecmp(icebergCatalogName, POSTGRES_CATALOG_NAME) == 0)
780780
icebergCatalogType = POSTGRES_CATALOG;
781781
else
782782
ereport(ERROR,

‎pg_lake_table/tests/pytests/test_iceberg_catalog_server.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,27 @@ def test_reject_create_server_type_object_store(superuser_conn, extension):
518518
superuser_conn.rollback()
519519

520520

521+
def test_reject_create_server_reserved_name(superuser_conn, extension):
522+
"""CREATE SERVER with a reserved catalog name (case-insensitive) is blocked."""
523+
reserved_names = [
524+
"Postgres",
525+
"OBJECT_STORE",
526+
"ReSt",
527+
]
528+
for name in reserved_names:
529+
err = run_command(
530+
f"""
531+
CREATE SERVER "{name}" TYPE 'rest'
532+
FOREIGN DATA WRAPPER iceberg_catalog
533+
""",
534+
superuser_conn,
535+
raise_error=False,
536+
)
537+
assert err is not None, f"Expected error for reserved name '{name}'"
538+
assert "reserved for the extension-owned catalog" in str(err)
539+
superuser_conn.rollback()
540+
541+
521542
def test_reject_alter_postgres_server(superuser_conn, extension):
522543
"""ALTER SERVER on the extension-owned 'postgres' server is blocked."""
523544
err = run_command(
@@ -627,6 +648,36 @@ def test_reject_rename_rest_server(superuser_conn, extension):
627648
superuser_conn.rollback()
628649

629650

651+
def test_reject_rename_to_reserved_name(superuser_conn, extension):
652+
"""Renaming a user-created server TO a reserved name is blocked."""
653+
run_command(
654+
"""
655+
CREATE SERVER tmp_rename_srv TYPE 'rest'
656+
FOREIGN DATA WRAPPER iceberg_catalog
657+
OPTIONS (rest_endpoint 'http://localhost:8181')
658+
""",
659+
superuser_conn,
660+
)
661+
for reserved in ["POSTGRES", "Object_Store", "REST"]:
662+
err = run_command(
663+
f'ALTER SERVER tmp_rename_srv RENAME TO "{reserved}"',
664+
superuser_conn,
665+
raise_error=False,
666+
)
667+
assert err is not None, f"Expected error for renaming to '{reserved}'"
668+
assert "reserved for the extension-owned catalog" in str(err)
669+
superuser_conn.rollback()
670+
run_command(
671+
"""
672+
CREATE SERVER tmp_rename_srv TYPE 'rest'
673+
FOREIGN DATA WRAPPER iceberg_catalog
674+
OPTIONS (rest_endpoint 'http://localhost:8181')
675+
""",
676+
superuser_conn,
677+
)
678+
superuser_conn.rollback()
679+
680+
630681
def test_reject_owner_change_postgres_server(superuser_conn, extension):
631682
"""ALTER SERVER ... OWNER TO on the extension-owned 'postgres' server is blocked."""
632683
err = run_command(

0 commit comments

Comments
 (0)