8000 Don't use Asserts to check for violations of replication protocol. · postgres/postgres@4745c11 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4745c11

Browse files
tglsfdctglsfdc
committed
Don't use Asserts to check for violations of replication protocol.
Using an Assert to check the validity of incoming messages is an extremely poor decision. In a debug build, it should not be that easy for a broken or malicious remote client to crash the logrep worker. The consequences could be even worse in non-debug builds, which will fail to make such checks at all, leading to who-knows-what misbehavior. Hence, promote every Assert that could possibly be triggered by wrong or out-of-order replication messages to a full test-and-ereport. To avoid bloating the set of messages the translation team has to cope with, establish a policy that replication protocol violation error reports don't need to be translated. Hence, all the new messages here use errmsg_internal(). A couple of old messages are changed likewise for consistency. Along the way, fix some non-idiomatic or outright wrong uses of hash_search(). Most of these mistakes are new with the "streaming replication" patch (commit 4648243), but a couple go back a long way. Back-patch as appropriate. Discussion: https://postgr.es/m/1719083.1623351052@sss.pgh.pa.us
1 parent 0cd8a55 commit 4745c11

File tree

2 files changed

+9
-2
lines changed

2 files changed

+9
-2
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1331,7 +1331,7 @@ ReorderBufferBuildTupleCidHash(ReorderBuffer *rb, ReorderBufferTXN *txn)
13311331
ent = (ReorderBufferTupleCidEnt *)
13321332
hash_search(txn->tuplecid_hash,
13331333
(void *) &key,
1334-
HASH_ENTER | HASH_FIND,
1334+
HASH_ENTER,
13351335
&found);
13361336
if (!found)
13371337
{

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,14 @@ apply_handle_commit(StringInfo s)
491491

492492
logicalrep_read_commit(s, &commit_data);
493493

494-
Assert(commit_data.commit_lsn == remote_final_lsn);
494+
if (commit_data.commit_lsn != remote_final_lsn)
495+
ereport(ERROR,
496+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
497+
errmsg_internal("incorrect commit LSN %X/%X in commit message (expected %X/%X)",
498+
(uint32) (commit_data.commit_lsn >> 32),
499+
(uint32) commit_data.commit_lsn,
500+
(uint32) (remote_final_lsn >> 32),
501+
(uint32) remote_final_lsn)));
495502

496503
/* The synchronization worker runs in single transaction. */
497504
if (IsTransactionState() && !am_tablesync_worker())

0 commit comments

Comments
 (0)
0