Skip to content

Commit 6234569

Browse files
peterepetere
committed
Improve CREATE SUBSCRIPTION option parsing
When creating a subscription with slot_name = NONE, we failed to check that also create_slot = false and enabled = false were set. This created an invalid subscription and could later lead to a crash if a NULL slot name was accessed. Add more checks around that for robustness. Reported-by: tushar <tushar.ahuja@enterprisedb.com>
1 parent ce55481 commit 6234569

File tree

4 files changed

+34
-1
lines changed

4 files changed

+34
-1
lines changed

‎src/backend/commands/subscriptioncmds.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,9 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
168168
false, 0, false);
169169
}
170170
else
171-
elog(ERROR, "unrecognized option: %s", defel->defname);
171+
ereport(ERROR,
172+
(errcode(ERRCODE_SYNTAX_ERROR),
173+
errmsg("unrecognized subscription parameter: %s", defel->defname)));
172174
}
173175

174176
/*
@@ -214,6 +216,16 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
214216
ereport(ERROR,
215217
(errcode(ERRCODE_SYNTAX_ERROR),
216218
errmsg("slot_name = NONE and create_slot = true are mutually exclusive options")));
219+
220+
if (enabled && !*enabled_given && *enabled)
221+
ereport(ERROR,
222+
(errcode(ERRCODE_SYNTAX_ERROR),
223+
errmsg("subscription with slot_name = NONE must also set enabled = false")));
224+
225+
if (create_slot && !create_slot_given && *create_slot)
226+
ereport(ERROR,
227+
(errcode(ERRCODE_SYNTAX_ERROR),
228+
errmsg("subscription with slot_name = NONE must also set create_slot = false")));
217229
}
218230
}
219231

‎src/backend/replication/logical/worker.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,15 @@ ApplyWorkerMain(Datum main_arg)
15521552

15531553
myslotname = MySubscription->slotname;
15541554

1555+
/*
1556+
* This shouldn't happen if the subscription is enabled, but guard
1557+
* against DDL bugs or manual catalog changes. (libpqwalreceiver
1558+
* will crash if slot is NULL.
1559+
*/
1560+
if (!myslotname)
1561+
ereport(ERROR,
1562+
(errmsg("subscription has no replication slot set")));
1563+
15551564
/* Setup replication origin tracking. */
15561565
StartTransactionCommand();
15571566
snprintf(originname, sizeof(originname), "pg_%u", MySubscription->oid);

‎src/test/regress/expected/subscription.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
5656
ERROR: slot_name = NONE and enabled = true are mutually exclusive options
5757
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true);
5858
ERROR: slot_name = NONE and create_slot = true are mutually exclusive options
59+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
60+
ERROR: subscription with slot_name = NONE must also set enabled = false
61+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
62+
ERROR: subscription with slot_name = NONE must also set create_slot = false
63+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
64+
ERROR: subscription with slot_name = NONE must also set enabled = false
5965
-- ok - with slot_name = NONE
6066
CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
6167
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
@@ -82,6 +88,8 @@ ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
8288
-- fail
8389
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
8490
ERROR: subscription "doesnotexist" does not exist
91+
ALTER SUBSCRIPTION testsub SET (create_slot = false);
92+
ERROR: unrecognized subscription parameter: create_slot
8593
\dRs+
8694
List of subscriptions
8795
Name | Owner | Enabled | Publication | Synchronous commit | Conninfo

‎src/test/regress/sql/subscription.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
4444
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false, create_slot = true);
4545
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true);
4646
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true);
47+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
48+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
49+
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
4750

4851
-- ok - with slot_name = NONE
4952
CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
@@ -64,6 +67,7 @@ ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
6467

6568
-- fail
6669
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
70+
ALTER SUBSCRIPTION testsub SET (create_slot = false);
6771

6872
\dRs+
6973

0 commit comments

Comments
 (0)