Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1f280e8

Browse files
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 1730a33 commit 1f280e8

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
@@ -501,7 +501,14 @@ apply_handle_commit(StringInfo s)
501501

502502
logicalrep_read_commit(s, &commit_data);
503503

504-
Assert(commit_data.commit_lsn == remote_final_lsn);
504+
if (commit_data.commit_lsn != remote_final_lsn)
505+
ereport(ERROR,
506+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
507+
errmsg_internal("incorrect commit LSN %X/%X in commit message (expected %X/%X)",
508+
(uint32) (commit_data.commit_lsn >> 32),
509+
(uint32) commit_data.commit_lsn,
510+
(uint32) (remote_final_lsn >> 32),
511+
(uint32) remote_final_lsn)));
505512

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

0 commit comments

Comments
 (0)