diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2021-06-12 12:59:15 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2021-06-12 12:59:15 -0400 |
| commit | 8b5055812cadd00a86a438fc0d8bafeba3c4d874 (patch) | |
| tree | 695f5fccda8d30df3752169979fc8bd133c098e5 /src | |
| parent | 45322bd9b9a33f0f3098d34b56f1ca250f14208a (diff) | |
| download | postgresql-8b5055812cadd00a86a438fc0d8bafeba3c4d874.tar.gz postgresql-8b5055812cadd00a86a438fc0d8bafeba3c4d874.zip | |
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 464824323), but a couple go back a long way.
Back-patch as appropriate.
Discussion: https://postgr.es/m/1719083.1623351052@sss.pgh.pa.us
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/replication/logical/reorderbuffer.c | 2 | ||||
| -rw-r--r-- | src/backend/replication/logical/worker.c | 9 |
2 files changed, 9 insertions, 2 deletions
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 52519326690..1351b33011f 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -1380,7 +1380,7 @@ ReorderBufferBuildTupleCidHash(ReorderBuffer *rb, ReorderBufferTXN *txn) ent = (ReorderBufferTupleCidEnt *) hash_search(txn->tuplecid_hash, (void *) &key, - HASH_ENTER | HASH_FIND, + HASH_ENTER, &found); if (!found) { diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index da748668ecc..1dbc0a4a607 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -559,7 +559,14 @@ apply_handle_commit(StringInfo s) logicalrep_read_commit(s, &commit_data); - Assert(commit_data.commit_lsn == remote_final_lsn); + if (commit_data.commit_lsn != remote_final_lsn) + ereport(ERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg_internal("incorrect commit LSN %X/%X in commit message (expected %X/%X)", + (uint32) (commit_data.commit_lsn >> 32), + (uint32) commit_data.commit_lsn, + (uint32) (remote_final_lsn >> 32), + (uint32) remote_final_lsn))); /* The synchronization worker runs in single transaction. */ if (IsTransactionState() && !am_tablesync_worker()) |