Don't use Asserts to check for violations of replication protocol.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 12 Jun 2021 16:59:15 +0000 (12:59 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 12 Jun 2021 16:59:15 +0000 (12:59 -0400)
commitfe6a20ce54cbbb6fcfe9f6675d563af836ae799a
tree79aab32108476f8a1a4c5bb7883616be468722af
parentc3652f976b7696a96a9c5606cc2d613af77e2e63
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
src/backend/replication/logical/reorderbuffer.c
src/backend/replication/logical/worker.c