From b55b408bf6a8351d468ff06bd92617aa3de6e277 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 30 Jul 2026 18:26:23 -0700 Subject: [PATCH v3 09/11] CREATE SUBSCRIPTION: do not construct conninfo unnecessarily. Still check that the creating user has USAGE privileges on the server, and that the FDW supports subscription connections. Addresses finding 1 in the report from the linked discussion. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com Discussion: https://postgr.es/m/e103ae8daf74485e0c0ebde297fae735d38f54d1.camel@j-davis.com Backpatch-through: 19 --- src/backend/commands/subscriptioncmds.c | 37 ++++++++++++++++------ src/backend/foreign/foreign.c | 4 +-- src/test/regress/expected/subscription.out | 4 +-- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index b91c07d208e..d52050282a5 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -677,8 +677,8 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, Datum values[Natts_pg_subscription]; Oid owner = GetUserId(); HeapTuple tup; - Oid serverid; - char *conninfo; + Oid serverid = InvalidOid; + char *conninfo = NULL; char originname[NAMEDATALEN]; List *publications; uint32 supported_opts; @@ -799,30 +799,47 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, ForeignServer *server; Assert(!stmt->conninfo); - conninfo = NULL; server = GetForeignServerByName(stmt->servername, false); - aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, owner, ACL_USAGE); + serverid = server->serverid; + + /* check USAGE privileges on server */ + aclresult = object_aclcheck(ForeignServerRelationId, serverid, owner, ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); /* check user mapping */ GetUserMappingExtended(owner, server->serverid, WARNING); - serverid = server->serverid; - conninfo = ForeignServerConnectionString(owner, server); + /* + * Check conninfo if connecting; otherwise only check that the + * server's FDW supports connections. + */ + if (opts.connect) + { + conninfo = ForeignServerConnectionString(owner, server); + walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser()); + } + else + { + ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid); + + if (!OidIsValid(fdw->fdwconnection)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("foreign-data wrapper \"%s\" does not support subscription connections", + fdw->fdwname), + errdetail("Foreign-data wrapper must be defined with CONNECTION specified."))); + } } else { Assert(stmt->conninfo); - serverid = InvalidOid; conninfo = stmt->conninfo; + walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser()); } - /* Check the connection info string. */ - walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser()); - publications = stmt->publication; /* Everything ok, form a new tuple. */ diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c index 73343f017b3..7ad8e8ee56b 100644 --- a/src/backend/foreign/foreign.c +++ b/src/backend/foreign/foreign.c @@ -209,9 +209,9 @@ ForeignServerConnectionString(Oid userid, ForeignServer *server) if (!OidIsValid(fdw->fdwconnection)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("foreign data wrapper \"%s\" does not support subscription connections", + errmsg("foreign-data wrapper \"%s\" does not support subscription connections", fdw->fdwname), - errdetail("Foreign data wrapper must be defined with CONNECTION specified."))); + errdetail("Foreign-data wrapper must be defined with CONNECTION specified."))); connection_datum = OidFunctionCall3(fdw->fdwconnection, ObjectIdGetDatum(userid), diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index e36f227129b..715c84afaa9 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -180,8 +180,8 @@ SET SESSION AUTHORIZATION regress_subscription_user3; -- warn, need user mapping, then fail, FDW doesn't support connections CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false); WARNING: user mapping not found for user "regress_subscription_user3", server "test_server" -ERROR: foreign data wrapper "test_fdw" does not support subscription connections -DETAIL: Foreign data wrapper must be defined with CONNECTION specified. +ERROR: foreign-data wrapper "test_fdw" does not support subscription connections +DETAIL: Foreign-data wrapper must be defined with CONNECTION specified. CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); RESET SESSION AUTHORIZATION; ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; -- 2.43.0