Content-Type: multipart/mixed; boundary="------------LGtEM0oH00NJq6s0mYAWUOFS"
Message-ID: <b99b6101-2f14-3796-3dfa-4a6cd7d4326d@enterprisedb.com>
Date: Sun, 14 Aug 2022 18:16:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Content-Language: en-US
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: bogus assert in logicalmsg_desc
Archived-At: 
 <https://www.postgresql.org/message-id/b99b6101-2f14-3796-3dfa-4a6cd7d4326d%40enterprisedb.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------LGtEM0oH00NJq6s0mYAWUOFS
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi,

while experimenting with logical messages, I ran into this assert in
logicalmsg_desc:

    Assert(prefix[xlrec->prefix_size] != '\0');

This seems to be incorrect, because LogLogicalMessage does this:

    xlrec.prefix_size = strlen(prefix) + 1;

So prefix_size includes the null byte, so the assert points out at the
first payload byte. And of course, the check should be "==" because we
expect the byte to be \0, not the other way around.

It's pretty simple to make this crash by writing a logical message where
the first payload byte is \0, e.g. like this:

    select pg_logical_emit_message(true, 'a'::text, '\x00'::bytea);

and then running pg_waldump on the WAL segment.

Attached is a patch addressing this. This was added in 14, so we should
backpatch to that version.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--------------LGtEM0oH00NJq6s0mYAWUOFS
Content-Type: text/x-patch; charset=UTF-8; name="logicalmsgdesc-fix.patch"
Content-Disposition: attachment; filename="logicalmsgdesc-fix.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2FjY2Vzcy9ybWdyZGVzYy9sb2dpY2FsbXNnZGVz
Yy5jIGIvc3JjL2JhY2tlbmQvYWNjZXNzL3JtZ3JkZXNjL2xvZ2ljYWxtc2dkZXNjLmMKaW5k
ZXggMDk5ZTExYTg0ZTcuLjA4ZTAzYWEzMGQxIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9h
Y2Nlc3Mvcm1ncmRlc2MvbG9naWNhbG1zZ2Rlc2MuYworKysgYi9zcmMvYmFja2VuZC9hY2Nl
c3Mvcm1ncmRlc2MvbG9naWNhbG1zZ2Rlc2MuYwpAQCAtMjgsNyArMjgsNyBAQCBsb2dpY2Fs
bXNnX2Rlc2MoU3RyaW5nSW5mbyBidWYsIFhMb2dSZWFkZXJTdGF0ZSAqcmVjb3JkKQogCQlj
aGFyCSAgICptZXNzYWdlID0geGxyZWMtPm1lc3NhZ2UgKyB4bHJlYy0+cHJlZml4X3NpemU7
CiAJCWNoYXIJICAgKnNlcCA9ICIiOwogCi0JCUFzc2VydChwcmVmaXhbeGxyZWMtPnByZWZp
eF9zaXplXSAhPSAnXDAnKTsKKwkJQXNzZXJ0KHByZWZpeFt4bHJlYy0+cHJlZml4X3NpemUg
LSAxXSA9PSAnXDAnKTsKIAogCQlhcHBlbmRTdHJpbmdJbmZvKGJ1ZiwgIiVzLCBwcmVmaXgg
XCIlc1wiOyBwYXlsb2FkICglenUgYnl0ZXMpOiAiLAogCQkJCQkJIHhscmVjLT50cmFuc2Fj
dGlvbmFsID8gInRyYW5zYWN0aW9uYWwiIDogIm5vbi10cmFuc2FjdGlvbmFsIiwK

--------------LGtEM0oH00NJq6s0mYAWUOFS--